diff --git a/docs/nimble_companion_trial.md b/docs/nimble_companion_trial.md index bd23a3a2d..8c012b47a 100644 --- a/docs/nimble_companion_trial.md +++ b/docs/nimble_companion_trial.md @@ -93,6 +93,7 @@ python3 -B test/test_heltec_v4_wifi_setup_page.py python3 -B test/test_firmware_ram.py python3 -B test/test_esp32_dram.py python3 -B test/test_shared_mota_queue.py +python3 -B test/test_companion_mqtt_stack.py pio test -e native -f test_companion_node_prefs \ -f test_ble_tx_stall_watchdog -f test_companion_frame_queue \ -f test_display_driver @@ -104,6 +105,9 @@ checks PIN authentication and stealth; and exercises 176-byte frames, MTU and notification subscription gating, retry and duplicate prevention. Embedded builds compile against the pinned real library. RAM reports retain the existing wireless budget; linked headroom is not a measurement of live free heap. +The MQTT save callback also has a compiler-enforced stack-frame regression +check: nested preference copies previously overflowed the V4 loop task when +saving MQTT settings through WebConfig, despite sufficient free heap. Hardware qualification must additionally verify pairing and reconnection, advertised addresses for every MAC policy, contact synchronization, WiFi/BLE diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index 3bbf0c0a0..02b39fc1e 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -3163,9 +3163,10 @@ void MyMesh::onConfigBatchEnd() { CompanionMqttSetupPortal::saveStoredConfig(_mqtt_prefs); if (_mqtt_started && _mqtt_bridge) _mqtt_bridge->end(); _mqtt_started = false; - MQTTPrefs verified; - _mqtt_configured = CompanionMqttSetupPortal::loadStoredConfig(verified); - if (_mqtt_configured) _mqtt_prefs = verified; + // loadStoredConfig only replaces its destination after validation. Pass + // the live preferences directly: another 2.8 KB copy here nests with the + // loader's scratch copy and NVS calls, overflowing the ESP32 loop stack. + _mqtt_configured = CompanionMqttSetupPortal::loadStoredConfig(_mqtt_prefs); // The standalone Companion setting remains canonical even if MQTT config // verification reloaded an older copy of this field. _mqtt_prefs.wifi_power_save = getCompanionWiFiPowerSave(); diff --git a/test/test_companion_mqtt_stack.py b/test/test_companion_mqtt_stack.py new file mode 100644 index 000000000..d401f4980 --- /dev/null +++ b/test/test_companion_mqtt_stack.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +"""Bound the real Companion MQTT save callback's additional stack use.""" +from pathlib import Path +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] +CALLBACK = "void MyMesh::onConfigBatchEnd()" +PREAMBLE = r""" +#include +void reloadCompanionWiFiPowerSave(); +uint8_t getCompanionWiFiPowerSave(); +struct MQTTBridge { void end(); }; +struct CompanionMqttSetupPortal { + static bool saveStoredConfig(const MQTTPrefs&); + static bool loadStoredConfig(MQTTPrefs&); +}; +struct MyMesh { + void onConfigBatchEnd(); + void syncWiFiPowerSaving(); + bool _wc_mqtt_dirty, _mqtt_started, _mqtt_configured; + MQTTBridge* _mqtt_bridge; + MQTTPrefs _mqtt_prefs; +}; +""" + + +def callback_source(): + source = (ROOT / "examples/companion_radio/MyMesh.cpp").read_text() + start = source.index(CALLBACK) + return source[start:source.index("void MyMesh::execCommand", start)] + + +class CompanionMqttStackTests(unittest.TestCase): + def compile_callback(self, source): + with tempfile.TemporaryDirectory(prefix="meshcore-mqtt-stack-") as temp: + path = Path(temp) / "callback.cpp" + path.write_text(PREAMBLE + source) + return subprocess.run([ + "c++", "-std=c++17", "-Os", "-fno-inline", + "-Werror=frame-larger-than=512", + "-DESP32=1", "-DWIFI_SSID=1", "-DWITH_MQTT_BRIDGE=1", + "-I", str(ROOT / "src"), "-c", str(path), + "-o", str(Path(temp) / "callback.o"), + ], text=True, capture_output=True) + + def test_save_callback_leaves_stack_for_nvs_and_validation(self): + # The loader already needs a full preference scratch copy. The save + # callback must not add another one to the ESP32's 8 KB loop stack. + result = self.compile_callback(callback_source()) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_guard_rejects_the_hardware_stack_overflow_regression(self): + source = callback_source() + call = "CompanionMqttSetupPortal::loadStoredConfig(_mqtt_prefs)" + self.assertIn(call, source) + source = source.replace("{", "{\n MQTTPrefs verified;", 1) + source = source.replace(call, "CompanionMqttSetupPortal::loadStoredConfig(verified)") + source = source.replace( + "// The standalone Companion setting", + "if (_mqtt_configured) _mqtt_prefs = verified;\n" + " // The standalone Companion setting", + ) + result = self.compile_callback(source) + self.assertNotEqual(result.returncode, 0) + self.assertIn("frame size", result.stderr) + + +if __name__ == "__main__": + unittest.main()