-
Notifications
You must be signed in to change notification settings - Fork 19
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
Restructure MQTT config distribution #234
Conversation
hikinggrass
commented
Jan 8, 2025
- publish manifests individually on retained topics instead in one big blob
- change module/impl config entry format to include its type, this removes the need for the whole config schema just to determine the type
- reorder the config handler before the ready handler in manager to reflect the order of operations taken during initialization of a module
- smaller optimizations
The same functionality can be achieved without first including the main config in the serialized config and then deleting it... Signed-off-by: Kai-Uwe Hermann <[email protected]> # Conflicts: # src/manager.cpp Signed-off-by: Kai-Uwe Hermann <[email protected]>
Skip storing it in a temporary variable Signed-off-by: Kai-Uwe Hermann <[email protected]>
…etain Previously this was sent to every module individually which isn't necessary Signed-off-by: Kai-Uwe Hermann <[email protected]>
Add type to parsed_config_map and remove config entry from manifest sent via MQTT Signed-off-by: Kai-Uwe Hermann <[email protected]>
Signed-off-by: Kai-Uwe Hermann <[email protected]>
…e is required Signed-off-by: Kai-Uwe Hermann <[email protected]>
Signed-off-by: Kai-Uwe Hermann <[email protected]>
Signed-off-by: Kai-Uwe Hermann <[email protected]>
…qtt-config # Conflicts: # src/manager.cpp Signed-off-by: Kai-Uwe Hermann <[email protected]>
retained topics: Could this lead to potential left-over problems of previous EVerest runs? In the default mosquitto config it is even stored permanently surviving reboots. Is it ensured that there can't be race conditions where e.g. one module subscribes to mqtt before the manager publishes the new config? |
Yes, and this is partially addressed with #233 (at least for successful starts, there is be more cleanup needed in cases the manager exits early, I'll address this over there). However the retained topics behavior is already in v0.19 and v0.19.1, so #233 is already an improvement as is
For configs this should not be an issue since a module explicitly requests a config from the manager and this doesn't involve retained topics. For manifest/interfaces/etc. there may be scenarios where this is a problem. It seems pretty stable thought (eg. I run the ocpp-tests of everest-core in parallel with all cores of my machine reliably all the time) but I will investigate this a bit further |
just a random thought: would it make sense to clear those topics as a last will from the manager? |
yes, sounds like a good idea. I plan on opening a PR with some signal handling for the manager that could then trigger this as well. During normal operation #233 will clean them up after every module is ready |
…qtt-config Signed-off-by: Kai-Uwe Hermann <[email protected]>
#234 changed the format of the config passed to a module which wasn't properly implemented in everestpy Signed-off-by: Kai-Uwe Hermann <[email protected]>
* Fix config parsing for everestpy #234 changed the format of the config passed to a module which wasn't properly implemented in everestpy * refactor duplicate code in everestjs and everestpy * Bump everest version to 0.20.2 --------- Signed-off-by: Kai-Uwe Hermann <[email protected]>