Skip to content
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

WiFi Mesh Update 2.2 #6280

Merged
merged 41 commits into from
Mar 14, 2021
Merged

WiFi Mesh Update 2.2 #6280

merged 41 commits into from
Mar 14, 2021

Conversation

aerlon
Copy link
Contributor

@aerlon aerlon commented Jul 10, 2019

Main changes:

  • Add new ESP-NOW mesh backend.
  • Add HelloEspnow.ino example to demonstrate the current ESP-NOW mesh backend features.
  • Deprecate the ESP8266WiFiMesh class in favour of the new ESP-NOW and TCP/IP backends.
  • Update the TCP/IP mesh backend to use the new lwIP version preprocessor flag and remove obsolete preprocessor flags.

The code is still a bit of a work in progress, but it will give you an idea of what the ESP-NOW backend can do. The ESP-NOW mesh backend API is not guaranteed to be stable yet. Below is what remains before the PR is complete:

- Add HelloEspnow.ino example to demonstrate the ESP-NOW mesh backend features.

- Deprecate the ESP8266WiFiMesh class in favour of the new ESP-NOW and TCP/IP backends.

- Update the TCP/IP mesh backend to use the new lwIP version preprocessor flag and remove obsolete preprocessor flags.
@devyte
Copy link
Collaborator

devyte commented Jul 14, 2019

Without looking at the change details, what is the state of backwards compatibility?
Is backwards compat really needed? I ask, because if you think it makes sense to drop backwards compat in favor of the new ESP-NOW backend, then we can do that and target this for core 3.0.0.
Or if it makes sense to maintain backwards compat, this can be targeted earlier for core 2.6.0.

@aerlon
Copy link
Contributor Author

aerlon commented Jul 18, 2019

@devyte As it is, the code is fully backwards compatible. The old API is still there and works the same way as before. The two new backends are handled via separate classes, so they do not interfere with existing code.

The old ESP8266WiFiMesh class is now only responsible for backwards compatibility, it is completely disconnected from the new code. Seeing how that is the case, in the long term, dropping backwards compatibility makes sense because this old class will lack features compared to the new ones. Right now however, there is no further development cost involved in adding backwards compatibility since it is already in place, and removing it in the future will essentially only require the removal of the ESP8266WiFiMesh class, so at least from my point of view there seems to be no added benefit to targeting 3.0.0 instead of 2.6.0.

earlephilhower and others added 3 commits July 28, 2019 12:17
- Add createPermanentConnections argument to attemptAutoEncryptingTransmission method.

- Reduce risk of misinterpreting acks by adding check for ack sender MAC.

- Reduce _encryptionRequestTimeoutMs from 500 ms to 300 ms since this should give enough (100 %) margin to the level where problems start appearing (150 ms timeout) and also save a lot of time in case of request failure.

- Improve comments.
@TD-er
Copy link
Contributor

TD-er commented Sep 15, 2019

What is the status of this WIP/PR?
What is still needed, are there any new hurdles found?

aerlon and others added 2 commits September 17, 2019 20:32
…connectionQueue and latestTransmissionOutcomes vectors.

- Deprecate NetworkInfo and TransmissionResult classes.

- Add single recipient transmission methods.

- Add a getCurrentMessage method to TcpIpMeshBackend to maintain feature parity when using single recipient transmission methods.

- Increase code abstraction level in transmission methods.

- Remove use of networkIndex except for in constructors, since it can change after each scan.

- Make Espnow backend require at least BSSID to connect, and the TcpIp backend require at least SSID.

- Make printAPInfo method take NetworkInfo as argument.

- Add new TransmissionOutcome class to replace obsolete TransmissionResult.

- Add _scanMutex.

- Improve code abstraction in HelloEspnow.ino.

- Update HelloEspnow.ino example to demonstrate the new features.

- Update and improve comments.
@aerlon
Copy link
Contributor Author

aerlon commented Sep 18, 2019

@TD-er I have uploaded the most recent version of the code so you can take a look. At the moment I'm working on the mesh network implementation. No new problems have been encountered so far, so with some luck the only real code issue left to fix after the mesh network is the HMAC BearSSL switch.

@jetbalsa
Copy link

Willing to add in a bounty for this project to be completed and put into the main SDK.

@aerlon
Copy link
Contributor Author

aerlon commented Oct 22, 2019

@jrwr An interesting proposal. A bounty could enable me to allocate more time to the PR, which means faster completion. It would also give you a say in setting development priorities. Perhaps we should have a chat on Gitter to talk more about the details?

@jetbalsa
Copy link

I have DM'd you on Gitter, For the greater good :)

@devyte
Copy link
Collaborator

devyte commented Oct 29, 2019

@aerlon what is the status here? Please don't forget to ping me when you think this is far enough along so that I can review.
We're less than a week away from release 2.6.0, per current plan. Do you think you can finalize by then? No hurry, we can push back if not.

@aerlon
Copy link
Contributor Author

aerlon commented Oct 29, 2019

@devyte A week may be a bit tight with my current ambitions, but I'll cut some features from the first implementation of the mesh example. That should give me a decent chance to get all the rest done in time.

@devyte
Copy link
Collaborator

devyte commented Oct 29, 2019

@aerlon sure thing, just please keep in mind that the review itself takes some time for back and forth.

…smissionSuccessful() methods static in order to match the underlying data storage.

- Make it possible to transfer elements directly between connectionQueues.

- Add defaultBSSID value.

- Fix bug where encrypted Espnow-connections expired 1 ms too late.

- Add MutexTracker::captureBan() functionality and use it in the espnowReceiveCallbackWrapper method to ensure a consistent mutex environment there.

- Rename acceptRequest to acceptRequests since several requests can be accepted, not just one.

- Reorganize EspnowMeshBackend.cpp.

- Split sendEspnowResponses() method into sendEspnowResponses() and sendPeerRequestConfirmations().

- Add sendStoredEspnowMessages() method to provide the same functionality as the previous version of sendEspnowResponses().

- Add logic for handling peerRequestConfirmations received at the same time as a peer request is being made, to avoid lockups when there are simultaneous cyclic peer requests.

- Add logic for handling simultaneous reciprocal peer requests.

- Include MAC addresses in HMAC calculations for peer requests and use HMAC for all unencrypted peer request messages, to make sure we receive valid MAC combinations.

- Add asserts to ensure ESP-NOW encryption integrity during code changes.

- Add estimatedMaxDuration argument to performEspnowMaintainance and related methods.

- Add methods to EncryptedConnectionData for setting peer MAC.

- Remove createEncryptionRequestMessage function from JsonTranslator since it is not used, to increase clarity.

- Add encryptedConnectionsSoftLimit() and related functionality.

- Add mutex to protect connectionQueue usage during attemptTransmission.

- Add _ongoingPeerRequestMac variable.

- Add reservedEncryptedConnections() method.

- Add TransmissionOutcomesUpdateHook() callback.

- Add constConnectionQueue() method to allow connectionQueue usage while connectionQueue mutex is active.

- Rearrange attemptAutoEncryptingTransmission argument order to increase efficiency.

- Add functionality for serializing the unencrypted ESP-NOW connection.

- Add some constness.

- Improve comments.

- Improve documentation.

- Update keywords.txt.
@aerlon
Copy link
Contributor Author

aerlon commented Oct 31, 2019

@devyte Indeed. I’ve uploaded the most recent version of the backend code now. Almost all code is in the final state (barring any bug fix), and can be reviewed. The only exceptions are the example ino files, and the Crypto.h/Crypto.cpp files (used for HMAC). Strings will also be moved to flash where appropriate.

@jetbalsa
Copy link

@aerlon Thanks for picking up the project again, Its one of the things the SDK was really missing was a good meshnet. I hope this is the start of something great

- Avoid single character String concatenations done via String literals instead of char literals, as this is inefficient because of temporary String creations (esp8266#6571).
…but are untested in large mesh networks. Encrypted broadcast support is currently experimental.

- Add BroadcastTransmissionRedundancy and related functionality to reduce the transmission loss during broadcasts. Broadcast transmissions are now re-transmitted once per default. Broadcast throughput halved per default.

- Add getSenderAPMac method.

- Add FloodingMesh example in the HelloMesh.ino file.

- Improve JSON identifier names.

- Improve comments.

- Improve documentation.
@aerlon
Copy link
Contributor Author

aerlon commented Nov 3, 2019

The first version of the mesh example has been added and is ready for review. Unencrypted broadcasts should work well, but are untested in large mesh networks. Encrypted broadcast support is currently experimental.

@jrwr Thanks!

…les.

- Generalize and improve JSON processing code.

- Prevent mesh passwords from containing " characters to avoid messing up the JSON processing.

- Improve documentation.
@devyte devyte modified the milestones: 2.7.1, 2.7.2 May 11, 2020
aerlon and others added 6 commits May 11, 2020 16:18
… new ConditionalPrinter, EspnowDatabase, EspnowConnectionManager, EspnowTransmitter and EspnowEncryptionBroker classes.

- Improve mutex handling.

- Move verifyEncryptionRequestHmac function from JsonTranslator to EspnowEncryptionBroker.

- Remove UtilityMethods.cpp.
… to the Arduino core, instead of the versions local to the mesh library.

- Rearrange class variables to minimize storage padding.

- Add protected getters for EspnowMeshBackend and MeshBackendBase components.

- Partially update README.md
@aerlon
Copy link
Contributor Author

aerlon commented May 18, 2020

@devyte All the issues you brought up have been addressed, so the code is ready for another review. I will work on the readme and documentation a bit more before it is completely ready for merge, but that should not affect the code.

aerlon and others added 3 commits June 5, 2020 21:56
…te use in composition.

- Call ResponseTransmittedHook after every response transmission attempt, instead of after every successful response transmission attempt.

- Improve documentation.

- Finalize README.md.

- Update keywords.txt.
@aerlon
Copy link
Contributor Author

aerlon commented Jun 5, 2020

Readme, documentation and keywords are now fully updated. The PR is ready for merge.

@earlephilhower earlephilhower changed the title WiFi Mesh Update 2.2 - WIP WiFi Mesh Update 2.2 Jun 21, 2020
@devyte
Copy link
Collaborator

devyte commented Jul 6, 2020

I've been working on this review for a while now, and I'm not even half way done. Pushing back to v3.

@devyte devyte modified the milestones: 2.7.2, 3.0.0 Jul 6, 2020
@d-a-v d-a-v added the alpha included in alpha release label Jul 16, 2020
@devyte
Copy link
Collaborator

devyte commented Mar 14, 2021

Nothing major jumped out at me in this last pass. Ok to merge from me.

@devyte devyte merged commit 49a0927 into esp8266:master Mar 14, 2021
@dok-net
Copy link
Contributor

dok-net commented Mar 15, 2021

@devyte Hi, I seem to run into a serious API mess-up here:
PolledTimeout.h:

IRAM_ATTR // fast
  bool expired()

libraries/ESP8266WiFiMesh/src/ExpiringTimeTracker.h:

class ExpiringTimeTracker : private esp8266::polledTimeout::oneShotMs
[...]
bool expired() const;

I think it's not immediately caught by the compiler due to inlining and probably expired() in master doing fine with a const this, but my PR #6843 gets completely broken by it. I ask for the derived class to be fixed.

More details:
Pr #6843 removes const from PolledTimeout::expiredOneShot() in order to fix some implicit restart behavior that I consider a bug in PolledTimeout. It's been such a long time since writing and reviewing the still unmerged PR that for more precise analysis, I'd have to do in-depth research. I stand by my recollection, for the time being :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants