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

waitResponse() check ambiguity #62

Open
sslupsky opened this issue Aug 14, 2019 · 2 comments
Open

waitResponse() check ambiguity #62

sslupsky opened this issue Aug 14, 2019 · 2 comments

Comments

@sslupsky
Copy link
Contributor

I think I have found an situation where there is ambiguity in waitResponse(). This method is used to parse the AT response from the modem. It defaults to parse for r1=LORA_OK ("+OK") or r2=LORA_ERROR ("+ERR"). There appears to be provision to add additional checks (r3, r4, r5).

The following line is typical of the parsing method used:

if (r1 && data.endsWith(r1)) {

A problem arises because the modem can return seven types of error message as shown here:

https://github.com/arduino/mkrwan1300-fw/blob/63787fe5ed8bd07119caba20d2065a26004b2261/Projects/Multi/Applications/LoRa/AT_Slave/src/command.c#L83-L93

Because the parsing method uses the .endswith() string method, and the string is checked with every new character, any error message will match the first "+ERR" and therefore, you cannot add additional error codes when you call waitResponse().

I think this needs to be fixed. It seems to me that the string comparison should be done only after the entire response is received from the modem?

@flhofer
Copy link

flhofer commented Apr 28, 2021

Good point. Maybe parse once a \r, +, " " or = is received. Actually, the same way the firmware is doing it. However, at the moment I don't think there is anything else evaluated but the mere ERR or OK ( == or != 1 is tested), so it wouldn't make that big of a change anyway.

@flhofer
Copy link

flhofer commented Apr 30, 2021

Should be done with #93 if pulled :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants