-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add ethernet interface support (OTA and Watchdog) for Portenta H7 #328
Conversation
Codecov Report
@@ Coverage Diff @@
## master #328 +/- ##
=======================================
Coverage 94.87% 94.87%
=======================================
Files 27 27
Lines 1113 1113
=======================================
Hits 1056 1056
Misses 57 57
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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 👍 Just got a couple of questions and small suggestions.
src/utility/ota/OTA-portenta-h7.cpp
Outdated
@@ -61,7 +64,13 @@ int portenta_h7_onOTARequest(char const * ota_url) | |||
watchdog_reset(); | |||
|
|||
/* Download the OTA file from the web storage location. */ | |||
int const ota_portenta_qspi_download_ret_code = ota_portenta_qspi.download(ota_url, true /* is_https */); | |||
MbedSocketClass * download_socket = static_cast<MbedSocketClass*>(&WiFi); | |||
#if defined (ARDUINO_PORTENTA_H7_M7) |
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.
Not sure if this ifdef is really needed? After all, should not this file be only compiled for Portenta H7?
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.
Yes, it is. The file is compiled also for NICLA VISION that does not have Ethernet. I think the best thing to do here is to rename the file into OTA-stm32-h7.cpp
to avoid confusion, but then we need also to rename Arduino_Portenta_OTA
library; or make a new library called Arduino_STM32_H7_OTA
and deprecate Arduino_Portenta_OTA
.
src/utility/watchdog/Watchdog.cpp
Outdated
@@ -63,6 +66,11 @@ static void samd_watchdog_reset() | |||
} | |||
} | |||
|
|||
static void samd_watchdog_enable_network_feed() | |||
{ | |||
WiFi.setFeedWatchdogFunc(watchdog_reset); |
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.
This function works for NINA? Not to say that it does not, I just never saw it before 😉
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.
Yes, we added this function with this PR arduino-libraries/WiFiNINA#186 one year ago
src/utility/watchdog/Watchdog.cpp
Outdated
@@ -154,4 +173,13 @@ void watchdog_reset() | |||
mbed_watchdog_reset(); | |||
#endif | |||
} | |||
|
|||
void watchdog_enable_network_feed(bool use_ethernet) |
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 how far is watchdog_enable_network_feed
different from watchdog_enable
. What are the advantages of one over the other? I don't think I get it.
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.
watchdog_enable
enables the mcu watchdog
watchdog_enable_network_feed
enables watchdog kiks/resets during network operations. This was introduced with:
Thanks @aentinger |
21974b0
to
c6a12f6
Compare
Cleanup and rebase: c6a12f6 should be dropped before merge. |
rebased to resolve conflicts |
The implementation is done as for WiFi so we can reuse the same define ARDUINO_PORTENTA_H7_WIFI_HAS_FEED_WATCHDOG_FUNC Move watchdog network feed configuration into Watchdog module
In this way we can also use the more meaningful define BOARD_HAS_ETHERNET
…ernet.h In this way we can also use the more meaningful define BOARD_HAS_ETHERNET
This board is an mbed platform + WiFiNINA so needs special handling for watchdog network feed.
Dropped c6a12f6 |
Memory usage change @ fdaac0f
Click for full report table
Click for full report CSV
|
Needs:
Ethernet: add possibility to configure timeout with manual configuration arduino/ArduinoCore-mbed#526 for Ethernet connection