Implement batch transmission and better testing features #453
Conversation
There was a problem hiding this comment.
Thank you so much for this! I made just a few comments within the code.
The build tests fail because the giant menu_a_la_carte example still expects the testingMode() function to exist (lines 3213 to 3223). The documentation checks fail because there's a typo in the documentation for dataPublisher::txBufferInit() on line 271 of dataPublisherBase.h.
src/LogBuffer.cpp
Outdated
There was a problem hiding this comment.
If the LogBuffer were converted to a circular buffer (FIFO), using it with endpoints that cannot receive more than one record at a time would be much easier. How difficult do you think it would be to convert it?
I don't need it for this PR, but I am looking toward future use.
There was a problem hiding this comment.
That should not be too difficult but some of the math might be subtle and it would make the API a little hairy. I'll play with it.
Serialize timestamps and variable values to the data buffer, then unserialize and format into JSON arrays. Send interval is controlled by sendEveryX variable. When data is buffered, 202 is returned to avoid possibly misleading statement that data is saved by the server. Offers considerable data and power savings. Device reports 3K RAM free with six variables and 8K buffer. making LogBuffer a member variable avoids the static initialization order fiasco: https://isocpp.org/wiki/faq/ctors#static-init-order
The modem might not be able to accept the data written to it if its transmit buffer is full. In that case, the portion not accepted needs to be retried later. If we retry too many times, give up so we don't get stuck in an infinite loop.
Avoids unnecessary time and power consumption when publishers just plan to buffer the data.
Send the first few data points logged after initialization immediately instead of buffering them until the programmed interval elapses. Allows verification of functionality during deployment.
Avoid repeatedly retrying if the server is down, while still retrying a couple times in case the connection is unreliable.
Allows field verification of logger functionality after deployment without undue delay. Leave default at 0 to allegedly reduce user confusion.
tpwrules
force-pushed
the
batch-transmission
branch
2 times, most recently
from
2b7e625
to
bea5837
Compare
Allows easy field verification of functionality as opposed to previous testing function which only served a purpose when attached to a computer (and consumed lots of flash). This behavior can be disabled and the old behavior restored by defining `MS_LOGGERBASE_BUTTON_BENCH_TEST` to be `true`.
Allows immediate transmission of data in buffer without loss before powering off and decomissioning a deployed datalogger.
Increases reliability of sending large transmissions.
Towers seem to take longer to connect the less recently a connection has been estasblished. Now that that's been expanded from 15 minutes to 8 hours, increase the timeout for that action to 4 minutes from 50 seconds.
tpwrules
force-pushed
the
batch-transmission
branch
from
bea5837
to
2c28908
Compare
|
Can you help me understand the timed out tests? Is there anything I need to/can do about that? I also see that some boards might need a smaller default buffer size to avoid running out of RAM? |
|
The GitHub actions stop running further tests in a group after one fails. So when the Mega failed because the program was too big for its RAM, the rest of the tests stopped. |
|
Okay, how do you recommend we solve that? Is there a way to do board specific defines? |
| uint32_t getRecordTimestamp(int record); | ||
|
|
||
| /** | ||
| * @brief Gets the value of a particular vaiable in a particular record. |
|
You can do board-specific defines by checking for which board type is already defined in the compiler: |
|
Do you know which boards have less than 16K of RAM? |
|
The Mega is the only one I'm testing. |
|
@SRGDamia1, We'll want start working on this soon, given we've queued the following into our v0.18 release. EnviroDIY performance improvements Milestone: |
aufdenkampe
added this to the v0.36: Batch transmission; Adafruit_ADS1X15 v2; Alphasense CO2 sensor milestone
aufdenkampe
removed this from the v0.36: Batch transmission; Adafruit_ADS1X15 v2; Alphasense CO2 sensor milestone
|
Closed in favor of #479 |
Requires modification to the MonitorMyWatershed site to work properly. This PR has been marked as a draft until those changes are made.
This implementation of the protocol reuses the logic for formatting single values and buffers values as floats so there should be absolutely no change in transmitted values.
It's unclear if the two timeout extension commits at the end are still needed. I would be fine dropping them for now. I would also be fine splitting the testing button changes into a separate PR.
Please use the GitHub PR review feature to suggest changes.