Skip to content

Commit

Permalink
Fix egregious SDI-12 error!
Browse files Browse the repository at this point in the history
Signed-off-by: Sara Damiano <sdamiano@stroudcenter.org>
  • Loading branch information
SRGDamia1 committed Feb 11, 2021
1 parent 5b9040c commit 28b58ca
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build_menu.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Build Examples
name: Build All Menu Configurations

# Triggers the workflow on push or pull request events
on: [push, pull_request]
Expand Down
5 changes: 3 additions & 2 deletions src/sensors/SDI12Sensors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ bool SDI12Sensors::getResults(void) {
MS_DBG(F(" <<<"), static_cast<char>(_SDI12Internal.read()));
// ^^ ignore the repeated SDI12 address

while (_SDI12Internal.available()) {
while (_SDI12Internal.available() && (millis() - start) < 3000) {
int c = _SDI12Internal.peek();
if (c == '-' || (c >= '0' && c <= '9') || c == '.') {
float result = _SDI12Internal.parseFloat(SKIP_NONE);
Expand All @@ -377,7 +377,8 @@ bool SDI12Sensors::getResults(void) {
resultsReceived++;
}
} else if (c >= 0 && c != '\r' && c != '\n') {
MS_DBG(F(" <<<"), static_cast<char>(_SDI12Internal.read()));
int c_read = _SDI12Internal.read();
MS_DBG(F(" <<<"), static_cast<char>(c_read));
} else { // no point -1's and new lines to debugging port
_SDI12Internal.read();
}
Expand Down

2 comments on commit 28b58ca

@neilh10
Copy link
Contributor

@neilh10 neilh10 commented on 28b58ca Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering was this for #346

Having really had to dig into this, and bring out a logic analyzer to understand it, I'm just wondering if there could be a comment example inline for what is being parsed.
So I saw this coming back from the LT500 instrument, that wasn't being parsed

1D0!1+0.10563+16.6166+0.24390

Its really nice to have the SDI12 subsystem extending its features, but what might help an understanding of the code is to have an easy transition from older code to newer, as its very hard to debug.

The 0.25.1 code from #367

        _SDI12Internal.read();  // ignore the repeated SDI12 address
        for (uint8_t i = 0; i < _numReturnedValues; i++) {
            float result = _SDI12Internal.parseFloat();

long parseInt(LookaheadMode lookahead = SKIP_ALL, char ignore = NO_IGNORE_CHAR);

and above in 0.27.5 its changed to _SDI12Internal.parseFloat(SKIP_NONE); so the parseing of characters has to be done for it.

So what I think worked for me .. at least with the first few parameters on a single line was

        while (0 < _SDI12Internal.available()) {
            int c = _SDI12Internal.peek();
            if (c == '-' || (c >= '0' && c <= '9') || c == '.'|| c == '+') {
                float result = _SDI12Internal.parseFloat();

In case its useful.
Just a note that this issue with the watchdog made it an order of magnitude more difficult to debug - but of course it did work to force it in to repetitive reset, rather than just hanging.
#344

@neilh10
Copy link
Contributor

@neilh10 neilh10 commented on 28b58ca Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me I did a protocol timeout to be more visible, but I didn't see it happen with the above.
Just an idea.

#define SDI12_PROTOCOL_TIMEOUT  10000
            if ((millis() - start) > SDI12_PROTOCOL_TIMEOUT ) {
                PRINTOUT("SDI12Sensors parse timeout",_SDI12Internal.readStringUntil('\n'));
                break;
            }

Please sign in to comment.