-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Allow watchdog kick while waiting for network #14
Conversation
Memory usage change @ b10b5e1
Click for full report table
Click for full report CSV
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some small things :-) Otherwise looks good
src/ArduinoCellular.cpp
Outdated
@@ -214,12 +217,17 @@ bool ArduinoCellular::awaitNetworkRegistration(){ | |||
if(this->debugStream != nullptr){ | |||
this->debugStream->println("Waiting for network registration..."); | |||
} | |||
while (!modem.waitForNetwork()) { | |||
while (!modem.waitForNetwork(20000L)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please extract this constant into a constexpr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our code we indent directives as if they were normal code to improve readabiltiy
Co-authored-by: Sebastian Romero <s.romero.zh@gmail.com>
Memory usage change @ d1c9d4a
Click for full report table
Click for full report CSV
|
Memory usage change @ 8691251
Click for full report table
Click for full report CSV
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
#if defined(ARDUINO_ARCH_MBED) | ||
#include "Watchdog.h" | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this flag also cover https://github.com/arduino/ArduinoCore-renesas and not just https://github.com/arduino/ArduinoCore-mbed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it is mbed only. This change is related to ArduinoIotCloud integration that enables watchdog for mbed and samd boards but not for the renesas one.
No description provided.