From 43c1b342bd33e0faae01f27ec6a18c38bd178f76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Fesser?= Date: Mon, 10 Aug 2026 00:28:30 +0200 Subject: [PATCH] fix: assign one telemetry channel per sensor reading --- repeater/handler_helpers/protocol_request.py | 84 +++++++------------ ...test_handler_helpers_path_protocol_text.py | 34 +------- 2 files changed, 36 insertions(+), 82 deletions(-) diff --git a/repeater/handler_helpers/protocol_request.py b/repeater/handler_helpers/protocol_request.py index 7322b4b..2b4f6ed 100644 --- a/repeater/handler_helpers/protocol_request.py +++ b/repeater/handler_helpers/protocol_request.py @@ -287,23 +287,13 @@ class ProtocolRequestHelper: # else 0.0 V (voltage-only floor, mirroring the companion self-telemetry). lpp = bytearray(encode_voltage(TELEM_CHANNEL_SELF, self._battery_voltage(readings))) - # INA219 power telemetry and environment sensors: the power telemetry - # channels are emitted first, then environment sensors continue on the - # next available channel. + # One channel per sensor reading (matches firmware channel assignment). if perm_mask & TELEM_PERM_ENVIRONMENT: channel = TELEM_CHANNEL_SELF + 1 - if self._has_sensor_data(readings, "bus_voltage_v"): - lpp.extend(encode_voltage(channel, self._battery_voltage(readings))) - channel += 1 - if self._has_sensor_data(readings, "current_ma"): - lpp.extend(encode_current(channel, self._battery_current(readings))) - channel += 1 - if self._has_sensor_data(readings, "power_mw"): - lpp.extend(encode_power(channel, self._battery_power(readings))) - channel += 1 - for reading in readings: - entry = self._encode_environment_reading(channel, reading) + entry = self._encode_power_reading( + channel, reading + ) + self._encode_environment_reading(channel, reading) if entry: lpp.extend(entry) channel += 1 @@ -342,45 +332,35 @@ class ProtocolRequestHelper: return 0.0 @staticmethod - def _has_sensor_data(readings, key: str) -> bool: - """Return True when any reading exposes the requested sensor field.""" - for reading in readings: - if not reading.get("ok"): - continue - data = reading.get("data") or {} - if data.get(key) is not None: - return True - return False + def _encode_power_reading(channel: int, reading) -> bytes: + """Encode a voltage/current/power reading as CayenneLPP, or b"" if none. - @staticmethod - def _battery_current(readings) -> float: - """First configured sensor's current in amps, else 0.000A.""" - for reading in readings: - if not reading.get("ok"): - continue - data = reading.get("data") or {} - current = data.get("current_ma") - if current is not None: - try: - return float(current) / 1000.0 - except (TypeError, ValueError): - continue - return 0.000 - - @staticmethod - def _battery_power(readings) -> int: - """First configured sensor's power in watts, else 0W.""" - for reading in readings: - if not reading.get("ok"): - continue - data = reading.get("data") or {} - power = data.get("power_mw") - if power is not None: - try: - return int(float(power) / 1000.0) - except (TypeError, ValueError): - continue - return 0 + TODO: a multi-rail sensor (e.g. INA3221) would need one channel per + rail here, not one channel for the whole reading. + """ + if not reading.get("ok"): + return b"" + data = reading.get("data") or {} + out = bytearray() + voltage = data.get("bus_voltage_v") + if voltage is not None: + try: + out.extend(encode_voltage(channel, float(voltage))) + except (TypeError, ValueError): + pass + current = data.get("current_ma") + if current is not None: + try: + out.extend(encode_current(channel, float(current) / 1000.0)) + except (TypeError, ValueError): + pass + power = data.get("power_mw") + if power is not None: + try: + out.extend(encode_power(channel, int(float(power) / 1000.0))) + except (TypeError, ValueError): + pass + return bytes(out) @staticmethod def _encode_environment_reading(channel: int, reading) -> bytes: diff --git a/tests/test_handler_helpers_path_protocol_text.py b/tests/test_handler_helpers_path_protocol_text.py index cc77ba7..bd3b90b 100644 --- a/tests/test_handler_helpers_path_protocol_text.py +++ b/tests/test_handler_helpers_path_protocol_text.py @@ -754,10 +754,10 @@ def test_telemetry_admin_full_mask_includes_environment_sensors(): expected = ( encode_voltage(TELEM_CHANNEL_SELF, 12.6) + encode_voltage(TELEM_CHANNEL_SELF + 1, 12.6) - + encode_current(TELEM_CHANNEL_SELF + 2, 0.5) - + encode_power(TELEM_CHANNEL_SELF + 3, 6) - + encode_temperature(TELEM_CHANNEL_SELF + 4, 21.5) - + encode_relative_humidity(TELEM_CHANNEL_SELF + 4, 55.0) + + encode_current(TELEM_CHANNEL_SELF + 1, 0.5) + + encode_power(TELEM_CHANNEL_SELF + 1, 6) + + encode_temperature(TELEM_CHANNEL_SELF + 2, 21.5) + + encode_relative_humidity(TELEM_CHANNEL_SELF + 2, 55.0) ) assert lpp == expected @@ -775,32 +775,6 @@ def test_telemetry_guest_forced_to_base_only(): assert lpp == encode_voltage(TELEM_CHANNEL_SELF, 0.0) -def test_telemetry_includes_ina219_power_channels_before_environment_sensors(): - """INA219 voltage/current/power values are emitted before env sensors.""" - sm = _FakeSensorManager( - [ - _reading(bus_voltage_v=12.6, current_ma=500.0, power_mw=6000.0), - _reading(temperature_c=21.5, humidity_pct=55.0), - ] - ) - helper = ProtocolRequestHelper( - identity_manager=MagicMock(), packet_injector=AsyncMock(), sensor_manager=sm - ) - admin = SimpleNamespace(is_guest=lambda: False) - - lpp = helper._handle_get_telemetry(admin, 0, b"\x00") - - expected = ( - encode_voltage(TELEM_CHANNEL_SELF, 12.6) - + encode_voltage(TELEM_CHANNEL_SELF + 1, 12.6) - + encode_current(TELEM_CHANNEL_SELF + 2, 0.5) - + encode_power(TELEM_CHANNEL_SELF + 3, 6) - + encode_temperature(TELEM_CHANNEL_SELF + 4, 21.5) - + encode_relative_humidity(TELEM_CHANNEL_SELF + 4, 55.0) - ) - assert lpp == expected - - def test_telemetry_inverse_mask_gates_environment(): """An inverse mask that clears the environment bit strips env sensors.