- Notifications
You must be signed in to change notification settings - Fork82
Add ethernet interface support (OTA and Watchdog) for Portenta H7#328
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecov-commenter commentedJul 27, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 ushow 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.
Uh oh!
There was an error while loading.Please reload this page.
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 intoOTA-stm32-h7.cpp to avoid confusion, but then we need also to renameArduino_Portenta_OTA library; or make a new library calledArduino_STM32_H7_OTA and deprecateArduino_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 PRarduino-libraries/WiFiNINA#186 one year ago
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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 iswatchdog_enable_network_feed different fromwatchdog_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 watchdogwatchdog_enable_network_feed enables watchdog kiks/resets during network operations. This was introduced with:
Thanks@aentinger |
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.
Droppedc6a12f6 |
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