Add MQTTManager unit test harness and CI

[jamesmulcahy]

This adds a harness for MQTTManager unit tests, runs them in CI, and uses it to cover the entity types, the EntityManager, OpenHAB and the config the manager sends to panels. The bugs the tests found are either fixed in their own commits or pinned as disabled tests.

Running the tests

cd docker/MQTTManager
./run_tests.sh                                   # build and run everything (in Docker)
./run_tests.sh --gtest_filter='NSPanelTest.*'    # extra arguments go to the test binary

The build runs in Docker (same python:3.12.5-bookworm base and compiler as the production image). The first run builds the Conan dependencies from source and takes about an hour; after that they are cached in the nspm-mqttmanager-conan volume and a rebuild plus run takes a few minutes. Output is also written to tests/last_run.log.

./compile_mqttmanager.sh --test still works: it now sets the new NSPM_BUILD_TESTS CMake option instead of sed-editing CMakeLists.txt.

The harness

• NSPM_BUILD_TESTS CMake option. It turns on TEST_MODE=1 and builds tests/. The old if (TEST_MODE==1) block compared a string and was always false, so test discovery never ran.

• tests/ builds its own nspm_mqttmanager_tests executable. test_helpers.hpp has scoped database rows (entities, scenes, panels) that remove themselves, and builders that write entity_data exactly as Django's rest.py does.

• Send capture, TEST_MODE only. MQTT_Manager, HomeAssistantManager and OpenhabManager record what they send, so tests check the exact MQTT messages and service calls without a live connection:

  • HomeAssistantManager::test_process_event() delivers an event through the real observer routing.
  • OpenhabManager gets hooks to feed a raw websocket message through its parser and to point its REST calls at an address without opening a websocket. tests/fake_http_server.hpp answers those REST calls from 127.0.0.1, using ixwebsocket's ix::HttpServer (already used by the Nextion image server, so no new dependency).
  • MQTT_Manager and CommandManager can report how many callbacks are attached, used to check that destroyed panels clean up.

  Production builds are unchanged.

• The existing inline tests in the Config and Light libraries are linked in whole, so they register too.

• CI in .github/workflows/mqttmanager-tests.yml runs on PRs and pushes to main/devel that touch docker/MQTTManager/.

  • The Conan package cache is a host directory saved with actions/cache, keyed on conanfile.py and the test Dockerfile. It's saved even if the tests fail, so only the first run (or a dependency change) pays the one-hour build.
  • Results are written as JUnit XML and published to the job summary with action-junit-report. It uses annotate_only, because creating a check run needs checks: write, which PRs from forks don't get.

What's covered

Area	File
Light config loading	light_config_test.cpp
Home Assistant switches, buttons, scenes and scripts, thermostats and media players	switch_test.cpp, button_test.cpp, scene_test.cpp, thermostat_test.cpp, media_player_test.cpp
NSPM buttons	button_test.cpp
EntityManager: entities, rooms and panels	entity_manager_test.cpp
OpenHAB switches, scenes and thermostats	openhab_test.cpp
The config sent to panels, and panel commands	nspanel_test.cpp

nspanel_test.cpp covers:

• Config contents: defaults, panel settings, temperature calibration, screensaver fallbacks, button modes, thermostat limits, relay group bindings, and room infos with and without locking.
• Config sending: reloads, and panels in an unknown room or that were denied (which get no config).
• Panel states: pending, accepted and denied.
• Commands: reboot topics, MQTT payload buttons, and detached buttons toggling an entity or activating a scene.
• Callback cleanup: when a panel is destroyed or renamed.

Fixes

Each fix is its own commit, with tests:

• Broken thermostat test blocks. home_assistant_thermostat.cpp and openhab_thermostat.cpp each ended with a copy of the Home Assistant light tests, which didn't compile under TEST_MODE. Removed. Production was unaffected.
• Unused settings key removed. OPENHAB_RGB_CHANNEL_NAME had no entry in _setting_key_map, which the existing inline config test caught. The setting is a leftover that nothing reads, so it's removed from MQTTManager, the Django settings view and the React settings store.
• Room temperature provider not initialised. Room::_room_temp_provider was never initialised, so every room without a temperature sensor logged "Got unknown temperature provider", and could detach an observer it had never attached.
• Wrong controller check for NSPM buttons. The NSPMButton constructor checked for the Home Assistant controller (copied from HomeAssistantSwitch), so every NSPM button logged "has not been recognized as controlled by HOME_ASSISTANT" when it loaded.
• Room temperature sensor not sent to panels (regression). d8220f6 dropped the line that sets inside_temperature_sensor_mqtt_topic. Since then, panels always show their built-in sensor on the screensaver, even when their room has a temperature sensor.
• Crash on a non-numeric thermostat limit. The limits are free text in the web interface and were parsed with std::stoi. A value like abc threw an exception that reload_config() doesn't catch. std::stoi also silently truncated 21.5 to 21 and read 21abc as 21. The limits are now parsed whole:
  • a value that isn't a number, or is out of range, logs an error and is sent as 0;
  • a decimal is rounded to the nearest degree, with a warning.
• NSPanel members not initialised. Humidity and pressure were sent to the web interface from uninitialised memory until the panel's first status report, and the relay state and "register as light" flags were never initialised. The relay 2 debug log also printed relay 1's setting.
• Pending panels reported as accepted. Since Feat first page react #391, the NSPanel constructor overwrote AWAITING_ACCEPT with WAITING. A newly discovered panel showed as "waiting" with "accepted": true. Accepting a panel had only worked because of this bug: register requests from panels awaiting accept or denied are ignored, and nothing moved an accepted panel out of those states. reload_config() now moves a newly accepted panel to WAITING.
• OpenHAB thermostat updates dropped at random. OpenhabThermostat ignores an event if the item changed less than a second ago, but the _last_*_change timestamps it compares against were never initialised. When that memory held a large value, OpenHAB updates for that item were silently ignored. CI hit this in follows_target_temperature_and_mode_from_openhab.
• OpenHAB light group events dropped at random. OpenhabLight's _openhab_group_*_thread_running flags were never initialised. If one started out true, the thread that processes that group's brightness, colour temperature or RGB events was never started.
• Use-after-free after deleting a panel. ~NSPanel() didn't detach its CommandManager callback, its nspanel/<mac>/log subscription, or the legacy nspanel/<name>/status and status_report subscriptions. After a panel was deleted, the next button press from any panel, or the next message on one of those topics, called into the destroyed object. These topics are now stored and detached. Renaming a panel also moves its legacy subscriptions to the new name instead of leaving the old name subscribed.

Known bugs, pinned as DISABLED_ tests

Each has a KNOWN BUG comment. Remove DISABLED_ when fixing it.

• LightConfig.DISABLED_non_string_controller_does_not_throw: a light whose controller isn't a string throws nlohmann::json::type_error instead of falling back to Home Assistant.
• OpenhabSceneTest.DISABLED_activating_sends_the_token: OpenhabScene::activate() builds the Authorization header from .c_str() of a temporary, so the header is a dangling pointer.
• OpenhabThermostatTest.DISABLED_set_preset_and_swing_command_their_items and DISABLED_fetches_the_initial_preset_and_swing_over_rest: the thermostat reads openhab_preset_item / openhab_swing_item / openhab_swingh_item, but Django stores *_mode_item. Presets and swing are sent to openhab/items//command and never follow OpenHAB. Once the names are fixed, the initial preset fetch compares against the HVAC mode, and the horizontal swing fetch checks the vertical item.
• OpenhabThermostatTest.DISABLED_publishes_the_current_temperature_from_openhab: the current temperature item is never attached, and its callback checks the target temperature item. Panels never show the thermostat's own reading.
• OpenhabThermostatTest.DISABLED_fetches_the_initial_target_temperature_over_rest: the initial target temperature fetched over REST is rounded to a whole degree, so 21.5 shows as 22.

Notes

• The tests share one SQLite database for the whole run. Rooms are never deleted, because each room runs a detached thread, so tests create rooms with unique names.
• The inline OpenHAB light tests take about 11 s, because they wait out the light's 1 s state debounce several times.
• Possible next steps: tests against a Django-migrated schema, and the websocket/STOMP command paths.

🤖 Generated with Claude Code

[jamesmulcahy]

Hi @tpanajott -- Here's another set of tests (and several fixes) for the MQTTManager side of things. It looks like a lot but it's 99% new test code, rather than changes to existing logic.

As issues were found, I kept the fixes (and associated tests to defend them) isolated to individual commits, to keep things clearer -- that's partially why there are so many commits. You're obviously free to squash them on merge.

[tpanajott]

Hey there. Sorry for the delay, it's just been a bit much in life and also these PRs are massive 😆 This looks pretty good but there are still some behaviours that I'd like to check though I'm not sure as to how to do that the best way. For example, requesting a specific brightness on the main page should behave differently depending on which lights are on, what their settings are and so on. Though, I suspect that might be better to do after this PR has landed.

[tpanajott]

This is awesome. I'll need to look into doing this when doing the actual release builds as well.

[tpanajott]

We can probably remove this and any reference to openhab_rgb_channel_name as it's a remnant of how we planned to do things in the beginning but it never worked out that way.

[jamesmulcahy]

Ack -- removed!

[tpanajott]

This all probably works but it's pretty hard to read. Would this not be better handed over to a library?

[jamesmulcahy]

Yeah, good call out -- this was pretty sloppy. Looks like ix::HttpServer, which is in a library we're already using, can be used for this. I've cut over to it

[jamesmulcahy]

Hey there. Sorry for the delay, it's just been a bit much in life and also these PRs are massive 😆

No worries -- I understand there's a lot to review here. No pressure on my side.

This looks pretty good but there are still some behaviours that I'd like to check though I'm not sure as to how to do that the best way. For example, requesting a specific brightness on the main page should behave differently depending on which lights are on, what their settings are and so on. Though, I suspect that might be better to do after this PR has landed.

Agreed; as far as I'm aware, this doesn't regress anything, but fixes some targeted issues identified during tests. If we want to expand coverage into other use cases, that's probably best done as extra PRs on top. It'll be good to get future PRs defended with a baseline of tests/builds and iterate from there.