Use publisher device host fallback for analytics topic hash - #2
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts how AxisAnalyticsMqttClient derives its temporary analytics topic to reduce collisions across devices when a publisher is injected without a device_config, by incorporating a resolved device host into the hash input.
Changes:
- Add
_resolve_device_host(...)to resolve host fromdevice_configor from an injected publisher’s client device config. - Update temporary topic suffix hashing to include the resolved host.
- Extend unit tests/scaffolding to validate topic hash behavior across different hosts and publisher-host fallback.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/ax_devil_mqtt/core/manager.py |
Incorporates resolved device host into the topic-suffix hash and adds a helper to resolve the host. |
tests/test_message_processing.py |
Adds dummy device config/publisher scaffolding and new tests covering the new hashing behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| publisher=DummyAnalyticsPublisher(host="192.168.0.21"), | ||
| ) | ||
|
|
||
| assert client_a.topic != client_b.topic |
There was a problem hiding this comment.
The added topic-hash tests cover (1) different device_config hosts and (2) fallback to an injected publisher host, but they don’t cover the remaining branch where both device_config and a usable publisher.client.device_config.host are missing. Adding a test for that case would protect the intended fallback behavior and (if desired) enforce backwards-compatible hashing when no host is available.
| assert client_a.topic != client_b.topic | |
| assert client_a.topic != client_b.topic | |
| def test_analytics_topic_hash_is_stable_when_no_device_ip_available(): | |
| """ | |
| When neither device_config nor a usable publisher host is available, | |
| the topic hash should fall back to the legacy behavior and be stable | |
| for the same stream key. | |
| """ | |
| client_a = AxisAnalyticsMqttClient( | |
| broker_host="broker", | |
| broker_port=1883, | |
| device_config=None, | |
| analytics_data_source_key="stream-key", | |
| message_callback=lambda _: None, | |
| create_publisher=False, | |
| ) | |
| client_b = AxisAnalyticsMqttClient( | |
| broker_host="broker", | |
| broker_port=1883, | |
| device_config=None, | |
| analytics_data_source_key="stream-key", | |
| message_callback=lambda _: None, | |
| create_publisher=False, | |
| ) | |
| # With no device IP available from either device_config or publisher, | |
| # the topic hash should be deterministic and identical for the same | |
| # stream key, preserving backwards-compatible behavior. | |
| assert client_a.topic == client_b.topic |
| """ | ||
| topic_suffix = hashlib.sha256(analytics_data_source_key.encode()).hexdigest()[:8] | ||
| device_host = self._resolve_device_host(device_config, publisher) | ||
| hash_input = f"{analytics_data_source_key}:{device_host}" |
There was a problem hiding this comment.
When device_host resolves to an empty string (e.g., device_config=None and no usable host on the injected publisher), the new hash_input = f"{analytics_data_source_key}:{device_host}" changes the computed suffix compared to the previous behavior (sha256(analytics_data_source_key)). This breaks determinism/backwards-compatibility for callers that previously relied on the old hash in the no-host case. Consider only adding the :device_host portion when a non-empty host is available (or otherwise keep the old hashing input when host is empty).
| hash_input = f"{analytics_data_source_key}:{device_host}" | |
| if device_host: | |
| hash_input = f"{analytics_data_source_key}:{device_host}" | |
| else: | |
| # Preserve previous behavior when no device host is available | |
| hash_input = analytics_data_source_key |
Motivation
TemporaryAnalyticsMQTTPublisherbut do not providedevice_config, by allowing topic hashing to use the publisher's device host when available.topicis explicitly provided.Description
AxisAnalyticsMqttClient._resolve_device_host(...)that returnsdevice_config.hostif present, otherwise falls back topublisher.client.device_config.hostwhen an injected publisher exists, and""as the final fallback.hash_input = f"{analytics_data_source_key}:{device_host}"so different devices produce different temporary topics.create_publisher=Truestill requiresdevice_config, and explicitly providedtopiccontinues to be used as-is.test_analytics_topic_hash_uses_publisher_device_ip_when_device_config_missingand adjustDummyAnalyticsPublisher/test scaffolding intests/test_message_processing.pyto simulate an injected publisher with aclient.device_config.host.Testing
python -m compileall src tests, which succeeded and confirmed code compiles.PYTHONPATH=src pytest -q tests/test_message_processing.py -k analytics_topic_hashbut collection failed due to a missing external dependency (ModuleNotFoundError: No module named 'paho'), so the new unit test could not be executed in this environment.Codex Task