Repository navigation
Fix display GPIO contention by switching to MQTT subscriber - #6
Merged
Merged
Conversation
Both weatherhat.service and weatherhat-display.service instantiated WeatherHAT() at startup. Under libgpiod v2 the IO Expander interrupt line (BCM4) is requested exclusively per-process, so the second service to start got `OSError: [Errno 16] Device or resource busy`, and the display process driving SPI DC in an unknown state produced banded pixel noise on the LCD instead of the UI. Reported in issue #4. The publisher is the hardware owner. The display now subscribes to the same MQTT topics the rest of the stack (Telegraf, HA, Grafana) already consumes — no new IPC layer, and button GPIO lines (BCM 5/6/16/24) are disjoint from the sensor interrupt so they can coexist with the publisher without contention. - bin/display-interface.py: replace SensorDataCollector with MqttSensorSubscriber (thread-safe, populates SensorState + history buffers from sensors/weather/# topics). Add stale/offline banner driven by the existing LWT status topic. - bin/mqtt-publisher.py: publish sensor values with retain=True so new subscribers (including the display on restart) get the latest value on connect rather than waiting PUBLISH_INTERVAL. - weatherhat-display.service: load config/mqtt.env so the display can reach the broker. - weatherhat/__init__.py: guard close() with getattr so __del__ on a partially-initialized instance doesn't mask the real error with an AttributeError about _poll_thread (secondary bug seen in the issue). - tests/test_weatherhat.py: regression test for the __del__ guard. Fixes #4
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
Fixes #4. Both
weatherhat.serviceandweatherhat-display.serviceinstantiatedWeatherHAT()at startup. Under libgpiod v2 the IO Expander interrupt line (BCM4) is requested exclusively per-process, so the second service to start hitOSError: [Errno 16] Device or resource busy, and the display process driving SPI DC in an unknown state produced banded pixel noise on the LCD instead of the UI.Architecture decision
Consulted the embedded-engineer and architect agents — both independently recommended Option B (MQTT subscriber display) over merging processes or adding an IPC layer. The embedded side confirmed the pin map is disjoint at the HAT level: only the publisher needs BCM4; display buttons (BCM 5/6/16/24) and display SPI pins don't conflict with anything the publisher owns. The architect side confirmed the display touches
WeatherHATfor exactly seven scalar values, all already published tosensors/weather/#— so blast radius is one file plus a retained-flag flip on the publisher.Changes
bin/display-interface.py— full refactor. Removedimport weatherhatandSensorDataCollector. AddedMqttSensorSubscriber(thread-safe, populatesSensorState+ history buffers fromsensors/weather/#topics on paho's network-loop thread). Added stale/offline banner driven by the existing LWTsensors/weather/statustopic and a2 × PUBLISH_INTERVALstaleness threshold.bin/mqtt-publisher.py—send_payloadnow publishes withretain=Trueso new subscribers (including the display after a restart) get the latest value on connect instead of waiting up toPUBLISH_INTERVAL. This is also the correct MQTT pattern for state topics and what Home Assistant expects.weatherhat-display.service— loadsconfig/mqtt.envviaEnvironmentFileso the display can reach the broker with the same credentials as the publisher.weatherhat/__init__.py—close()now usesgetattrguards so__del__on a partially-initialized instance doesn't mask the real error withAttributeError: 'WeatherHAT' object has no attribute '_poll_thread'(the secondary traceback seen in the issue report).tests/test_weatherhat.py— regression test: simulateSMBus()raising during__init__and verifyclose()on the bare instance does not raise.Depends on
This works on top of already-deployed Pis (update.sh reinstalls the local package), but for fresh installs it depends on PR #5 (issue #3 fix) landing first so that
install-service.shputs the localweatherhatpackage in the venv — otherwisefrom weatherhat import historyindisplay-interface.pyfails.Test plan
EBUSY, no banded noise, LCD shows live dataweatherhat.service→ display shows "PUBLISHER OFFLINE" banner within one render tickWhat I considered and rejected