MKRWAN icon indicating copy to clipboard operation
MKRWAN copied to clipboard

waitResponse() check ambiguity

Open sslupsky opened this issue 5 years ago • 2 comments

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:

https://github.com/arduino-libraries/MKRWAN/blob/2fa519d6bd8aceb359e3c3fcff91f0bd4b4324be/src/MKRWAN.h#L855

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?

sslupsky avatar Aug 14 '19 18:08 sslupsky

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 avatar Apr 28 '21 10:04 flhofer

Should be done with #93 if pulled :)

flhofer avatar Apr 30 '21 00:04 flhofer