Move SARA_R5 features #10
Conversation
LARA_R6 does not support these AT commands, as per #4 Moved into SARA-R5 class for now, but if other modules support these, they should be moved into a sub-class that can be inherited like the voice class
Should just use macros to not have duplicate definitions Also remove PSD and UTIME definitions due to existence in sara_r5.h
| @@ -42,125 +43,119 @@ | |||
|
|
|||
| // ## Suported AT Commands | |||
| // ### General | |||
| const char SARA_R5_COMMAND_AT[] = "AT"; // AT "Test" | |||
There was a problem hiding this comment.
For all these constant defines:
const char constWhich is : const data type ('the string') and const pointer ('the pointer')
not just
const charWhich is just saying this symbol points to 'constant data'
We all make this mistake.
And if I was being really pedantic - I'd use const char * const - the use of "[]" to declare a pointer array is valid, but off standard.
There was a problem hiding this comment.
Fine with me, though I'll do that separately from this PR
src/sfe_sara_r5.cpp
Outdated
| if (command == nullptr) | ||
| return UBX_CELL_ERROR_OUT_OF_MEMORY; | ||
| if (mode == UBX_CELL_UTIME_MODE_STOP) // stop UTIME does not require a sensor | ||
| sprintf(command, "%s=%d", SARA_R5_GNSS_REQUEST_TIME, mode); |
There was a problem hiding this comment.
use snprintf - throughout this file
There was a problem hiding this comment.
I assume the same should be done in the other files, correct? I'll make a separate issue for this, since the scope of this PR is really just to move these functions
There was a problem hiding this comment.
Yes - buffer overflows are painful, using proper string functions help.
| @@ -0,0 +1,360 @@ | |||
| #include "sfe_sara_r5.h" | |||
There was a problem hiding this comment.
File should use 4 char spacing, not 2
There was a problem hiding this comment.
Will do that as part of addressing #9 before the v1 branch is merged into main. Easy enough to run all the code through a formatter at the end
| if (err == UBX_CELL_ERROR_SUCCESS) | ||
| { | ||
| int mStore, sStore, scanned = 0; | ||
| char *searchPtr = strstr(response, "+UTIME:"); |
There was a problem hiding this comment.
I assume the same should be done in the other files, correct? I'll make a separate issue for this, since the scope of this PR is really just to move these functions
Fixes #4
Not sure if it's best to move these features into
sfe_sara_r5.hor an intermediate class like the voice class. If the latter is preferred, please suggest a name for it, because I don't know what to call it 🙃