Clarify and de-duplicate the event system (#102)#109
Merged
Conversation
events.py (EventEmitter) is the sole delivery mechanism; mqtt_events.py provides the MqttClientEvents name registry and typed dataclass payloads it carries. Internal emit call sites in state_tracker.py, client.py and subscriptions.py now reference the MqttClientEvents constants instead of duplicating raw event-name strings, so each event name is defined once. Update module docstrings and the event-system reference docs to document the relationship, and add tests covering typed-payload delivery through the emitter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #102's premise is partly inaccurate:
events.pyandmqtt_events.pyare not two competing event mechanisms.EventEmitter(events.py) is the sole delivery mechanism;mqtt_events.pyonly supplies the event-name registry (MqttClientEvents) and the typed dataclass payloads carried by each event. They are complementary. This PR documents that relationship and removes the one genuine duplication.Changes
emitcall sites inmqtt/state_tracker.py,mqtt/client.pyandmqtt/subscriptions.pypreviously hardcoded raw event-name strings ("temperature_changed","connection_resumed", etc.) that were also defined asMqttClientEventsconstants. They now reference the constants, so each event name is defined in exactly one place.events.pyandmqtt_events.pymodule docstrings and indocs/reference/python_api/events.rst.tests/test_mqtt_events.pycovers the registry (get_all_events,get_event_value) and verifies that subscribing with aMqttClientEventsconstant delivers the matching typed payload through the emitter.No mechanism was removed; there is no behavior change. Per the discussion, a full rewrite/merge was not warranted because the two modules were never duplicate mechanisms.
Validation
scripts/lint.py(ruff + pyright): passedmypy src/nwp500: no issues in 50 filespytest: 621 passedNote: this PR is stacked on #101 (base branch
pr-101-awscrt-wrap) and should merge after it.Closes #102
Recreated because #107 was auto-closed when its stacked base branch (pr-101-awscrt-wrap) was deleted after PR #105 merged. Retargeted to main.