Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Main header doesn't include MemoryStream.hpp #4

Open
skyhisi opened this issue Jul 17, 2019 · 6 comments
Open

Main header doesn't include MemoryStream.hpp #4

skyhisi opened this issue Jul 17, 2019 · 6 comments
Labels
enhancement New feature or request

Comments

@skyhisi
Copy link

skyhisi commented Jul 17, 2019

The main header file doesn't include MemoryStream.hpp.

Fix:

--- StreamUtils.h.orig  2019-06-05 08:30:33.000000000 +0100
+++ StreamUtils.h       2019-07-17 21:20:23.643851000 +0100
@@ -10,6 +10,7 @@
 #include "StreamUtils/Prints/BufferingPrint.hpp"
 #include "StreamUtils/Prints/LoggingPrint.hpp"
 #include "StreamUtils/Streams/LoggingStream.hpp"
+#include "StreamUtils/Streams/MemoryStream.hpp"
 #include "StreamUtils/Streams/ReadBufferingStream.hpp"
 #include "StreamUtils/Streams/ReadLoggingStream.hpp"
 #include "StreamUtils/Streams/ReadThrottlingStream.hpp"
@bblanchon
Copy link
Owner

Hi @skyhisi,

MemoryStream is neither in the header nor in the documentation nor in the examples.
I use this class in the tests, but I didn't want to publish it, because I'm pretty sure users will complain about its fixed capacity.

@skyhisi, do you really use this class in your project?

Best Regards,
Benoit

@skyhisi
Copy link
Author

skyhisi commented Jul 18, 2019

Hi Benoit,
I found it whilst browsing the source, and it seems to work fine to me.

My use case is to build up a string in memory with print to serialise up some data, before reading it out the buffer in blocks for sending over the network. I know the maximum size of the data I have, so a fixed size is fine for my use. Using MemoryStream was simpler than managing a buffer manually with sprintf and strcat 😃

Something like:

MemoryStream stream(512);
stream.print(F("temperature="));
stream.print(temperature);
stream.print(F(",humidity="));
stream.print(humidity);
// ...
char buf[64];
while ((len = buffer.readBytes(buf, sizeof(buf))) != 0)
{
  client.write((uint8_t*)buf, len);
}

If there's a better way of doing this, let me know.
Thanks,
Silas

@bblanchon
Copy link
Owner

Did you mean stream.readBytes() instead of buffer.readBytes()?

If so, you probably want to use BufferingPrint, WriteBufferingStream, or WriteBufferingClient, because that's exactly what they do (assuming that client is an instance of Client).

WriteBufferingClient bufferingClient(client, 64);
bufferingClient.print(F("temperature="));
bufferingClient.print(temperature);
bufferingClient.print(F(",humidity="));
bufferingClient.print(humidity);
bufferingClient.flush();

@skyhisi
Copy link
Author

skyhisi commented Jul 18, 2019

Yes, sorry I did mean stream.readBytes(), I was trying to simplify my code into the example and missed changing that.

In this case, I'm not sure I can use the normal buffering classes, as I need to get the size of the complete buffer (I'm using available) before anything is sent to the client.

My actual code is closer to this:

PubSubClient mqttClient(/*...*/);

void upload()
{
  // ...
  MemoryStream influxLine(512);
  influxLine.print(F("weather "));

  influxLine.print(F("temperature="));
  influxLine.print(temperature);
  //...

  if (!mqttClient.beginPublish(TOPIC_PREFIX "/influx", influxLine.available(), true))
  {
    LOG("Failed to begin publish Influx update");
    goto exit;
  }
  while ((len = influxLine.readBytes(buf, 64)) != 0)
  {
    mqttClient.write((uint8_t*)buf, len);
  }
  if (!mqttClient.endPublish())
  {
    LOG("Failed to end publish Influx update");
    goto exit;
  }
// ...

Is there a better way to do this?

@bblanchon
Copy link
Owner

I would probably use sprintf_P() in a similar situation, but this is alright too.

I think you're right: this class seems to be useful outside of my unit tests.
As I told you, my main concern is that users will ask for an elastic capacity, so maybe I should rename it to FixedMemoryStream.
What do you think?

@skyhisi
Copy link
Author

skyhisi commented Jul 19, 2019

FixedMemoryStream is a good name. It allows in the future to have DynamicMemoryStream & StaticMemoryStream versions if required, which could be like the DynamicJsonDocument and StaticJsonDocument classes (variable size / compile time size).

@bblanchon bblanchon added the enhancement New feature or request label Jul 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants