-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
change(esp_now_serial): No teardown on retry limit #10293
Conversation
After max retries is met once the ESP_NOW_Serial_Class performs "end()". This removes the peer from ESP_NOW. Further messages to and from ESP_NOW_Serial are not received or sent. Peer should stay in ESP_NOW to re-establish connection even with data loss. This change will "retry and drop" the data piece by piece instead of aborting the connection.
👋 Hello ClockeNessMnstr, we appreciate your contribution to this project! Click to see more instructions ...
Review and merge process you can expect ...
|
This seems to stop ESP_NOW_Serial::write from returning 0 although it returns "1" for the first attempt anyway. |
Actually it returns "1" until it does the "teardown" even though it did not succeed in sending the first byte(s). This is not due to my change. |
The idea makes sense, @P-R-O-C-H-Y please make sure it's implemented in the best way. |
ac97fa4
to
51ff298
Compare
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
Test Results 56 files 56 suites 4m 14s ⏱️ Results for commit 0ee66a8. ♻️ This comment has been updated with latest results. |
51ff298
to
d0ae1cd
Compare
@ClockeNessMnstr Hi, I did a small change to your PR so we have backward compatibility if someone want to remove the peer on fail. So we have it as optional. By default the peer won't be removed. Can you please test if it's ok for you? Thanks |
Description of Change
After max retries is met once the ESP_NOW_Serial_Class performs "end()" which removes the peer from ESP_NOW.
Further messages to and from ESP_NOW_Serial are not received or sent.
Peer should stay in ESP_NOW to re-establish connection even with data loss.
This change will "retry and drop" the data piece by piece instead of aborting the connection.
ESP_NOW_Serial "Peer" will not remove itself from ESP_NOW_Peer list when data sent is not received by a peer. ESP_NOW_Serial::end() is still called when ESP_NOW::send() fails.
Tests scenarios
Tested on a pair of M5Stack Stamp ESP32-C3.
Confirmed that transmission is able to continue after: disconnecting the Receiving module from power.
Attempting to send data.
Restoring power to Receiving module.
Previously, transmission halts as the cleanup/teardown in ESP_NOW_Serial::end() is triggered.
Related links
I did not create an issue for this.
Issue was NowSerial had to be re-initialized essentially for every byte without knowing if the peer was present/restored.
Workaround was to use broadcast address for TX and a separate ESP_NOW_Serial peer with MAC address for RX.