Move Wi-Fi code to strategy pattern#534
Conversation
|
After a quit short look at the design/flow: I'm not sure if we want to keep that layout of the direct call to network functions from the sensor class. It would be probably better to implement a buffer type handling of the sending/receiving data. For receiving would also make a buffer, and nack (not sure if that is currently possible in the feature flags) when it is full. The buffer allows to handle the commands at a given time. And not without delay interrupting stuff. We also need to care about disconnect/changing protocol. I see there a high likely hood of getting nullptr references. (.comms() = nullptr for a short of time) So what i think would be better is the following: The current implementation assumes the network is all the time faster than the IMU preparing data. But with ESP-Now I'm not sure if this always is the case. Well also seems also not always the case with WiFi. |
It's the strategy pattern (https://refactoring.guru/design-patterns/strategy)
The beauty of this is that it doesn't matter much. You can implement a buffering strategy that gets the data from the tracker, buffers it up (or more realistically, only keeps the latest information) and either hands that over to the corresponding strategy or those access that instead. Though I'm doubtful that either wifi or espnow would be too slow to handle our workload.
We barely receive anything, I don't think there's a point. We would also need to manually NACK, since this is UDP.
I don't see a reason to ever set that to a nullptr. If a disconnection happens, the strategy is kept around to reconnect, see: |
ok, still not fan of it, but ok.
I agree, but i also hope that it will be easier to handle packetbundle stuff that way, even if we don't gather all sensor info in 1 loop. Currently we send the packets into a udp buffer that will be sent out anytime. I would love to be able to have a little control over that. Not sure if we can on esp8266_arduino maybe i have to dig deeper there.
hm maybe it does not matter. but when then i would add the same as sending
hm ok no problem with multicore? |
This PR does most of the legwork to decouple the Wi-Fi implementation from the rest of the codebase. This should allow for easier implementation of alternative connection/communication methods, like ESP-Now.
Two major refactors were done. First, the Wi-Fi handler code was remade using the state pattern, which leads to a slightly cleaner implementation. Secondly the Wi-Fi code was encapsulated into a strategy pattern implementation.
In theory creating a new implementation of the CommunicationsStrategy interface, alongside the appropriate code for handling that new strategy in the network manager class should allow for alternative communication methods. It's likely that further changes will need to be done to the interface for other classes to work as expected, but I tried making it as basic as possible.
This PR should not change any functionality in a significant way.