From a786edffcd797142c6e92dcb2d97caa2bfca2fdc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 23:01:19 +0000 Subject: [PATCH] portduino Sensors: add air quality types Sensors: entries can now name AirQualityMetrics fields (pm25_standard, pm100_standard, particle counts, co2, formaldehyde, VOC/NOx index, ...) as well as EnvironmentMetrics fields. - CommandSensorFields gains an AirQualityMetrics table and messageFor(), which says which telemetry message a Type belongs to. No field name appears in both. pm_status_flags is left out: it is a status bitmask, not a measurement. - Each telemetry module owns one CommandSensor that runs only the entries for its message, so no command runs twice. The air-quality module registers its instance once (its i2cScanFinished() runs twice) and first, so it is read last and wins a shared field. - Air-quality readings follow the air-quality module's own enable switch and interval. The boot warning now counts unread entries per module. - --check's info line and config-dist.yaml say which setting each kind of type needs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011C7D9dE4aUYBrGdpPyHs25 --- bin/config-dist.yaml | 11 +- bin/test-config-check.sh | 6 +- src/modules/Modules.cpp | 33 +++- src/modules/Telemetry/AirQualityTelemetry.cpp | 13 ++ .../Telemetry/EnvironmentTelemetry.cpp | 2 +- .../Telemetry/Sensor/CommandSensor.cpp | 27 ++- src/modules/Telemetry/Sensor/CommandSensor.h | 11 +- .../Telemetry/Sensor/CommandSensorFields.cpp | 166 +++++++++++------- .../Telemetry/Sensor/CommandSensorFields.h | 9 +- src/platform/portduino/ConfigCheck.cpp | 5 +- test/fixtures/portduino-config/README.md | 2 +- test/fixtures/portduino-config/sensors.yaml | 6 +- test/test_command_sensor/test_main.cpp | 98 ++++++++++- 13 files changed, 292 insertions(+), 97 deletions(-) diff --git a/bin/config-dist.yaml b/bin/config-dist.yaml index 476b388d0a..fdd9f23799 100644 --- a/bin/config-dist.yaml +++ b/bin/config-dist.yaml @@ -271,16 +271,19 @@ HostMetrics: # Channel: 0 # channel to send Host Metrics over. Defaults to the primary channel. # UserStringCommand: cat /sys/firmware/devicetree/base/serial-number # Command to execute, to send the results as the userString -# Environment telemetry values read by running a command. Each command must print one number, in the -# units of the named EnvironmentMetrics field (temperature, relative_humidity, barometric_pressure, -# lux, wind_speed, ...). Read only while environment measurement is enabled in the Telemetry module -# config, and sent at its update interval. Commands are killed after 2 seconds. +# Telemetry values read by running a command. Each command must print one number, in the units of the +# named EnvironmentMetrics field (temperature, relative_humidity, barometric_pressure, lux, ...) or +# AirQualityMetrics field (pm25_standard, pm100_standard, co2, ...). Environment types are read only while +# environment measurement is enabled in the Telemetry module config, air-quality types only while air +# quality is, and each is sent at that setting's update interval. Commands are killed after 2 seconds. #Sensors: # - Type: temperature # Command: cat /sys/bus/iio/devices/iio:device0/in_temp_input # Scale: 0.001 # Optional multiplier; sysfs usually reports milli-units # - Type: relative_humidity # Command: /usr/local/bin/read-humidity +# - Type: co2 +# Command: /usr/local/bin/read-co2 Security: # Public keys authorized to send admin messages to this node, as base64, the same text the apps and diff --git a/bin/test-config-check.sh b/bin/test-config-check.sh index 2d3d00c25c..b9224c0935 100755 --- a/bin/test-config-check.sh +++ b/bin/test-config-check.sh @@ -178,8 +178,12 @@ assert "admin keys append across config.d" 0 adminkeys-append/config.yaml yaml \ echo echo "command sensors:" assert "a list of command sensors is clean" 0 sensors.yaml check \ - "Sensors are only read while environment measurement is enabled" \ + "Sensors are only read while the Telemetry module setting for their Type is enabled" \ "Result: 0 errors, 0 warnings" +# An air-quality Type is accepted like an environment one, and written back under its field name. +assert "air-quality sensor types are accepted" 0 sensors.yaml yaml \ + "Type: co2" \ + "Type: relative_humidity" assert "an unknown sensor Type is refused and the valid ones named" 1 sensors-bad-type.yaml check \ "Sensors Type 'humidity' is not an environment metric" \ "relative_humidity" \ diff --git a/src/modules/Modules.cpp b/src/modules/Modules.cpp index 6821301230..0ab7db4f09 100644 --- a/src/modules/Modules.cpp +++ b/src/modules/Modules.cpp @@ -70,6 +70,7 @@ #include "PortduinoGlue.h" #include "modules/DMShell.h" #include "modules/Telemetry/HostMetrics.h" +#include "modules/Telemetry/Sensor/CommandSensorFields.h" #if !MESHTASTIC_EXCLUDE_STOREFORWARD #include "modules/StoreForwardModule.h" #endif @@ -127,6 +128,29 @@ #if defined(HAS_HARDWARE_WATCHDOG) #include "watchdog/watchdogThread.h" #endif +#if ARCH_PORTDUINO && HAS_TELEMETRY && HAS_SENSOR && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR +// A Sensors: entry is read only by the telemetry module its Type belongs to; say so when that module is off. +static void warnUnreadCommandSensors() +{ + unsigned environment = 0; + unsigned airQuality = 0; + for (const auto &sensor : portduino_config.command_sensors) { + if (CommandSensorFields::messageFor(sensor.type) == CommandSensorFields::Message::AirQuality) + airQuality++; + else + environment++; + } + if (environment && !(moduleConfig.telemetry.environment_measurement_enabled || ENVIRONMENTAL_TELEMETRY_MODULE_ENABLE)) + LOG_WARN("config.yaml lists %u environment Sensors, but environment measurement is disabled, so they are not read", + environment); +#if !MESHTASTIC_EXCLUDE_AIR_QUALITY_SENSOR + if (airQuality && !(moduleConfig.telemetry.air_quality_enabled || AIR_QUALITY_TELEMETRY_MODULE_ENABLE)) + LOG_WARN("config.yaml lists %u air quality Sensors, but air quality telemetry is disabled, so they are not read", + airQuality); +#endif +} +#endif + /** * Create module instances here. If you are adding a new module, you must 'new' it here (or somewhere else) */ @@ -244,18 +268,15 @@ void setupModules() (moduleConfig.telemetry.environment_measurement_enabled || moduleConfig.telemetry.environment_screen_enabled)) { environmentTelemetryModule = new EnvironmentTelemetryModule(); } -#if ARCH_PORTDUINO - if (!portduino_config.command_sensors.empty() && - !(moduleConfig.telemetry.environment_measurement_enabled || ENVIRONMENTAL_TELEMETRY_MODULE_ENABLE)) - LOG_WARN("config.yaml lists %u Sensors, but environment measurement is disabled, so they are not read", - static_cast(portduino_config.command_sensors.size())); -#endif #if HAS_TELEMETRY && HAS_SENSOR && !MESHTASTIC_EXCLUDE_AIR_QUALITY_SENSOR if (moduleConfig.has_telemetry && (moduleConfig.telemetry.air_quality_enabled || moduleConfig.telemetry.air_quality_screen_enabled)) { new AirQualityTelemetryModule(); } #endif +#if ARCH_PORTDUINO + warnUnreadCommandSensors(); +#endif #if !MESHTASTIC_EXCLUDE_HEALTH_TELEMETRY if (nodeTelemetrySensorsMap[meshtastic_TelemetrySensorType_MAX30102].first > 0 || nodeTelemetrySensorsMap[meshtastic_TelemetrySensorType_MLX90614].first > 0) { diff --git a/src/modules/Telemetry/AirQualityTelemetry.cpp b/src/modules/Telemetry/AirQualityTelemetry.cpp index bfb0ab73bb..a550cfa67b 100644 --- a/src/modules/Telemetry/AirQualityTelemetry.cpp +++ b/src/modules/Telemetry/AirQualityTelemetry.cpp @@ -41,6 +41,9 @@ static constexpr uint16_t TX_HISTORY_KEY_AIR_QUALITY_TELEMETRY = 0x8004; #if __has_include() #include "Sensor/HM330XSensor.h" #endif +#if ARCH_PORTDUINO +#include "Sensor/CommandSensor.h" +#endif void AirQualityTelemetryModule::i2cScanFinished(ScanI2C *i2cScanner) { @@ -110,6 +113,16 @@ void AirQualityTelemetryModule::i2cScanFinished(ScanI2C *i2cScanner) } // order by priority of metrics/values (low top, high bottom) +#if ARCH_PORTDUINO + // Not an I2C device. Only added on the first scan; added first so it is read last and wins a shared field. + if (firstTime) { + auto *commandSensor = new CommandSensor(CommandSensorFields::Message::AirQuality); + if (commandSensor->initDevice(nullptr, nullptr)) + sensors.push_front(commandSensor); + else + delete commandSensor; + } +#endif addSensor(i2cScanner, ScanI2C::DeviceType::PMSA003I); addSensor(i2cScanner, ScanI2C::DeviceType::SEN5X); addSensor(i2cScanner, ScanI2C::DeviceType::SEN6X); diff --git a/src/modules/Telemetry/EnvironmentTelemetry.cpp b/src/modules/Telemetry/EnvironmentTelemetry.cpp index 626ab605f5..3e6c39c453 100644 --- a/src/modules/Telemetry/EnvironmentTelemetry.cpp +++ b/src/modules/Telemetry/EnvironmentTelemetry.cpp @@ -296,7 +296,7 @@ void EnvironmentTelemetryModule::i2cScanFinished(ScanI2C *i2cScanner) #if !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR #if ARCH_PORTDUINO // Not an I2C device. - commandSensor = new CommandSensor(); + commandSensor = new CommandSensor(CommandSensorFields::Message::Environment); if (commandSensor->initDevice(nullptr, nullptr)) { sensors.push_front(commandSensor); } else { diff --git a/src/modules/Telemetry/Sensor/CommandSensor.cpp b/src/modules/Telemetry/Sensor/CommandSensor.cpp index 30190d5b6c..d226cd46af 100644 --- a/src/modules/Telemetry/Sensor/CommandSensor.cpp +++ b/src/modules/Telemetry/Sensor/CommandSensor.cpp @@ -1,9 +1,8 @@ #include "configuration.h" -#if ARCH_PORTDUINO && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR +#if ARCH_PORTDUINO && (!MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR || !MESHTASTIC_EXCLUDE_AIR_QUALITY_SENSOR) #include "CommandSensor.h" -#include "CommandSensorFields.h" #include "UptimeClock.h" #include "mesh/Throttle.h" #include @@ -15,7 +14,10 @@ static constexpr uint32_t COMMAND_SENSOR_REFRESH_MS = 30 * 1000; static constexpr uint32_t COMMAND_SENSOR_POLL_MS = 100; static constexpr size_t COMMAND_SENSOR_MAX_QUOTED_OUTPUT = 64; -CommandSensor::CommandSensor() : TelemetrySensor(meshtastic_TelemetrySensorType_SENSOR_UNSET, "Command") {} +CommandSensor::CommandSensor(CommandSensorFields::Message message) + : TelemetrySensor(meshtastic_TelemetrySensorType_SENSOR_UNSET, "Command"), message(message) +{ +} CommandSensor::~CommandSensor() { @@ -26,13 +28,16 @@ CommandSensor::~CommandSensor() bool CommandSensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev) { - if (portduino_config.command_sensors.empty()) + for (const auto &sensor : portduino_config.command_sensors) + if (CommandSensorFields::messageFor(sensor.type) == message) + configs.push_back(sensor); + if (configs.empty()) return false; - configs = portduino_config.command_sensors; readings.assign(configs.size(), Reading()); loggedGenerations.assign(configs.size(), 0); - LOG_INFO("Init sensor: %s, %u from config.yaml Sensors", sensorName, static_cast(configs.size())); + LOG_INFO("Init sensor: %s, %u %s entries from config.yaml Sensors", sensorName, static_cast(configs.size()), + message == CommandSensorFields::Message::AirQuality ? "air quality" : "environment"); status = 1; initialized = true; worker = std::thread(&CommandSensor::run, this); @@ -115,8 +120,14 @@ bool CommandSensor::getMetrics(meshtastic_Telemetry *measurement) else LOG_WARN("Sensor %s: command %s: %s", sensor.type.c_str(), reading.problem.c_str(), sensor.command.c_str()); } - if (reading.valid && CommandSensorFields::apply(measurement->variant.environment_metrics, sensor.type, reading.value)) - any = true; + if (!reading.valid) + continue; + if (message == CommandSensorFields::Message::Environment && + measurement->which_variant == meshtastic_Telemetry_environment_metrics_tag) + any = CommandSensorFields::apply(measurement->variant.environment_metrics, sensor.type, reading.value) || any; + else if (message == CommandSensorFields::Message::AirQuality && + measurement->which_variant == meshtastic_Telemetry_air_quality_metrics_tag) + any = CommandSensorFields::apply(measurement->variant.air_quality_metrics, sensor.type, reading.value) || any; } return any; } diff --git a/src/modules/Telemetry/Sensor/CommandSensor.h b/src/modules/Telemetry/Sensor/CommandSensor.h index 036b431b2e..bd8ac8790e 100644 --- a/src/modules/Telemetry/Sensor/CommandSensor.h +++ b/src/modules/Telemetry/Sensor/CommandSensor.h @@ -2,9 +2,10 @@ #include "configuration.h" -#if ARCH_PORTDUINO && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR +#if ARCH_PORTDUINO && (!MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR || !MESHTASTIC_EXCLUDE_AIR_QUALITY_SENSOR) #include "../mesh/generated/meshtastic/telemetry.pb.h" +#include "CommandSensorFields.h" #include "PortduinoGlue.h" #include "TelemetrySensor.h" #include @@ -13,13 +14,14 @@ #include #include -// Fills EnvironmentMetrics fields from the shell commands listed under Sensors: in config.yaml. +// Fills telemetry fields from the shell commands listed under Sensors: in config.yaml. Each telemetry module +// owns one instance, which handles only the entries whose Type belongs to that module's message. // Commands run on a worker thread, so a slow or hung one never stalls the main loop; getMetrics() only // reads the latest results. class CommandSensor : public TelemetrySensor { public: - CommandSensor(); + explicit CommandSensor(CommandSensorFields::Message message); ~CommandSensor() override; virtual bool getMetrics(meshtastic_Telemetry *measurement) override; virtual bool initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev) override; @@ -36,7 +38,8 @@ class CommandSensor : public TelemetrySensor std::string problem; // why the last read gave no value; empty when valid }; - std::vector configs; // copied at init; read-only after + const CommandSensorFields::Message message; + std::vector configs; // this module's entries, copied at init; read-only after std::mutex mu; std::vector readings; // guarded by mu std::vector loggedGenerations; // main thread only diff --git a/src/modules/Telemetry/Sensor/CommandSensorFields.cpp b/src/modules/Telemetry/Sensor/CommandSensorFields.cpp index fbc9833aa0..156f4fc07b 100644 --- a/src/modules/Telemetry/Sensor/CommandSensorFields.cpp +++ b/src/modules/Telemetry/Sensor/CommandSensorFields.cpp @@ -13,11 +13,9 @@ namespace CommandSensorFields namespace { -using Setter = void (*)(meshtastic_EnvironmentMetrics &, float); - -struct Field { +template struct Field { const char *name; - Setter set; + void (*set)(Metrics &, float); }; template T roundAndClamp(float value) @@ -30,97 +28,147 @@ template T roundAndClamp(float value) return static_cast(rounded); } -#define FLOAT_FIELD(f) \ - {#f, [](meshtastic_EnvironmentMetrics &m, float v) { \ +#define FLOAT_FIELD(M, f) \ + {#f, [](M &m, float v) { \ m.f = v; \ m.has_##f = true; \ }} -#define INT_FIELD(f) \ - {#f, [](meshtastic_EnvironmentMetrics &m, float v) { \ +#define INT_FIELD(M, f) \ + {#f, [](M &m, float v) { \ m.f = roundAndClamp(v); \ m.has_##f = true; \ }} -const Field kFields[] = { - FLOAT_FIELD(temperature), - FLOAT_FIELD(relative_humidity), - FLOAT_FIELD(barometric_pressure), - FLOAT_FIELD(gas_resistance), - FLOAT_FIELD(voltage), - FLOAT_FIELD(current), - INT_FIELD(iaq), - FLOAT_FIELD(distance), - FLOAT_FIELD(lux), - FLOAT_FIELD(white_lux), - FLOAT_FIELD(ir_lux), - FLOAT_FIELD(uv_lux), - INT_FIELD(wind_direction), - FLOAT_FIELD(wind_speed), - FLOAT_FIELD(weight), - FLOAT_FIELD(wind_gust), - FLOAT_FIELD(wind_lull), - FLOAT_FIELD(radiation), - FLOAT_FIELD(rainfall_1h), - FLOAT_FIELD(rainfall_24h), - INT_FIELD(soil_moisture), - FLOAT_FIELD(soil_temperature), - FLOAT_FIELD(adc_voltage_ch0), - FLOAT_FIELD(adc_voltage_ch1), - FLOAT_FIELD(adc_voltage_ch2), - FLOAT_FIELD(adc_voltage_ch3), - FLOAT_FIELD(adc_voltage_ch4), - FLOAT_FIELD(adc_voltage_ch5), - FLOAT_FIELD(adc_voltage_ch6), - FLOAT_FIELD(adc_voltage_ch7), - FLOAT_FIELD(one_wire_temperature_ch0), - FLOAT_FIELD(one_wire_temperature_ch1), - FLOAT_FIELD(one_wire_temperature_ch2), - FLOAT_FIELD(one_wire_temperature_ch3), - FLOAT_FIELD(one_wire_temperature_ch4), - FLOAT_FIELD(one_wire_temperature_ch5), - FLOAT_FIELD(one_wire_temperature_ch6), - FLOAT_FIELD(one_wire_temperature_ch7), - INT_FIELD(lightning_strike_count_1h), - FLOAT_FIELD(lightning_distance_km), +using Env = meshtastic_EnvironmentMetrics; +const Field kEnvironmentFields[] = { + FLOAT_FIELD(Env, temperature), + FLOAT_FIELD(Env, relative_humidity), + FLOAT_FIELD(Env, barometric_pressure), + FLOAT_FIELD(Env, gas_resistance), + FLOAT_FIELD(Env, voltage), + FLOAT_FIELD(Env, current), + INT_FIELD(Env, iaq), + FLOAT_FIELD(Env, distance), + FLOAT_FIELD(Env, lux), + FLOAT_FIELD(Env, white_lux), + FLOAT_FIELD(Env, ir_lux), + FLOAT_FIELD(Env, uv_lux), + INT_FIELD(Env, wind_direction), + FLOAT_FIELD(Env, wind_speed), + FLOAT_FIELD(Env, weight), + FLOAT_FIELD(Env, wind_gust), + FLOAT_FIELD(Env, wind_lull), + FLOAT_FIELD(Env, radiation), + FLOAT_FIELD(Env, rainfall_1h), + FLOAT_FIELD(Env, rainfall_24h), + INT_FIELD(Env, soil_moisture), + FLOAT_FIELD(Env, soil_temperature), + FLOAT_FIELD(Env, adc_voltage_ch0), + FLOAT_FIELD(Env, adc_voltage_ch1), + FLOAT_FIELD(Env, adc_voltage_ch2), + FLOAT_FIELD(Env, adc_voltage_ch3), + FLOAT_FIELD(Env, adc_voltage_ch4), + FLOAT_FIELD(Env, adc_voltage_ch5), + FLOAT_FIELD(Env, adc_voltage_ch6), + FLOAT_FIELD(Env, adc_voltage_ch7), + FLOAT_FIELD(Env, one_wire_temperature_ch0), + FLOAT_FIELD(Env, one_wire_temperature_ch1), + FLOAT_FIELD(Env, one_wire_temperature_ch2), + FLOAT_FIELD(Env, one_wire_temperature_ch3), + FLOAT_FIELD(Env, one_wire_temperature_ch4), + FLOAT_FIELD(Env, one_wire_temperature_ch5), + FLOAT_FIELD(Env, one_wire_temperature_ch6), + FLOAT_FIELD(Env, one_wire_temperature_ch7), + INT_FIELD(Env, lightning_strike_count_1h), + FLOAT_FIELD(Env, lightning_distance_km), +}; + +// pm_status_flags is a sensor status bitmask, not a measurement, so it is not offered. +using Aq = meshtastic_AirQualityMetrics; +const Field kAirQualityFields[] = { + INT_FIELD(Aq, pm10_standard), INT_FIELD(Aq, pm25_standard), + INT_FIELD(Aq, pm40_standard), INT_FIELD(Aq, pm100_standard), + INT_FIELD(Aq, pm10_environmental), INT_FIELD(Aq, pm25_environmental), + INT_FIELD(Aq, pm100_environmental), INT_FIELD(Aq, particles_03um), + INT_FIELD(Aq, particles_05um), INT_FIELD(Aq, particles_10um), + INT_FIELD(Aq, particles_25um), INT_FIELD(Aq, particles_40um), + INT_FIELD(Aq, particles_50um), INT_FIELD(Aq, particles_100um), + FLOAT_FIELD(Aq, particles_tps), INT_FIELD(Aq, co2), + FLOAT_FIELD(Aq, co2_temperature), FLOAT_FIELD(Aq, co2_humidity), + FLOAT_FIELD(Aq, form_formaldehyde), FLOAT_FIELD(Aq, form_humidity), + FLOAT_FIELD(Aq, form_temperature), FLOAT_FIELD(Aq, pm_temperature), + FLOAT_FIELD(Aq, pm_humidity), FLOAT_FIELD(Aq, pm_voc_idx), + FLOAT_FIELD(Aq, pm_nox_idx), }; #undef FLOAT_FIELD #undef INT_FIELD -const Field *findField(const std::string &type) +std::string lowercase(const std::string &type) { std::string lower = type; for (auto &c : lower) c = static_cast(std::tolower(static_cast(c))); - for (const auto &field : kFields) + return lower; +} + +template const Field *findIn(const Field (&fields)[N], const std::string &type) +{ + const std::string lower = lowercase(type); + for (const auto &field : fields) if (lower == field.name) return &field; return nullptr; } +template +bool applyIn(const Field (&fields)[N], Metrics &metrics, const std::string &type, float value) +{ + const Field *field = findIn(fields, type); + if (!field) + return false; + field->set(metrics, value); + return true; +} + } // namespace std::string canonicalType(const std::string &type) { - const Field *field = findField(type); - return field ? field->name : ""; + if (const auto *field = findIn(kEnvironmentFields, type)) + return field->name; + if (const auto *field = findIn(kAirQualityFields, type)) + return field->name; + return ""; +} + +Message messageFor(const std::string &type) +{ + if (findIn(kEnvironmentFields, type)) + return Message::Environment; + if (findIn(kAirQualityFields, type)) + return Message::AirQuality; + return Message::None; } std::string knownTypes() { std::string out; - for (const auto &field : kFields) + for (const auto &field : kEnvironmentFields) out += (out.empty() ? "" : ", ") + std::string(field.name); + for (const auto &field : kAirQualityFields) + out += ", " + std::string(field.name); return out; } bool apply(meshtastic_EnvironmentMetrics &metrics, const std::string &type, float value) { - const Field *field = findField(type); - if (!field) - return false; - field->set(metrics, value); - return true; + return applyIn(kEnvironmentFields, metrics, type, value); +} + +bool apply(meshtastic_AirQualityMetrics &metrics, const std::string &type, float value) +{ + return applyIn(kAirQualityFields, metrics, type, value); } bool parseValue(const std::string &output, float &value) diff --git a/src/modules/Telemetry/Sensor/CommandSensorFields.h b/src/modules/Telemetry/Sensor/CommandSensorFields.h index 03a3329962..ad06c791df 100644 --- a/src/modules/Telemetry/Sensor/CommandSensorFields.h +++ b/src/modules/Telemetry/Sensor/CommandSensorFields.h @@ -7,18 +7,25 @@ #include "mesh/generated/meshtastic/telemetry.pb.h" #include -// The EnvironmentMetrics fields a Sensors: entry in config.yaml can fill, named as in the protobuf. +// The EnvironmentMetrics and AirQualityMetrics fields a Sensors: entry in config.yaml can fill, named as in +// the protobuf. No field name appears in both messages. namespace CommandSensorFields { +// Which telemetry message, and so which telemetry module, a type belongs to. +enum class Message { None, Environment, AirQuality }; + // Canonical (lowercase) field name for type, matched case-insensitively; empty if no field matches. std::string canonicalType(const std::string &type); +Message messageFor(const std::string &type); + // Every accepted type, comma separated, for error messages. std::string knownTypes(); // Stores value in the named field and sets its has_ flag. Integer fields are rounded and clamped. bool apply(meshtastic_EnvironmentMetrics &metrics, const std::string &type, float value); +bool apply(meshtastic_AirQualityMetrics &metrics, const std::string &type, float value); // Reads one finite number from command output; surrounding whitespace is allowed, anything else is not. bool parseValue(const std::string &output, float &value); diff --git a/src/platform/portduino/ConfigCheck.cpp b/src/platform/portduino/ConfigCheck.cpp index 4f6ebfbd4d..71f1f8a7d6 100644 --- a/src/platform/portduino/ConfigCheck.cpp +++ b/src/platform/portduino/ConfigCheck.cpp @@ -1409,8 +1409,9 @@ void checkMergedConfig(const PathIndex &paths, std::vector &findings) " times. Every command runs, but only one value can be sent"}); if (!portduino_config.command_sensors.empty()) findings.push_back({kInfo, merged, 0, - "Sensors are only read while environment measurement is enabled in the Telemetry module " - "config, and are then broadcast at its update interval"}); + "Sensors are only read while the Telemetry module setting for their Type is enabled " + "(environment measurement, or air quality for air-quality types), and are then broadcast " + "at that setting's update interval"}); } void printSummary() diff --git a/test/fixtures/portduino-config/README.md b/test/fixtures/portduino-config/README.md index c79d31d478..0df30a0e61 100644 --- a/test/fixtures/portduino-config/README.md +++ b/test/fixtures/portduino-config/README.md @@ -192,7 +192,7 @@ booting a node that silently never reports that reading. | File | Expected | | ------------------------- | ------------------------------------------------------------------------------------------------------ | -| `sensors.yaml` | Two sensors, one Type in mixed case. Clean apart from the info line about enabling them. | +| `sensors.yaml` | Three sensors, one an air-quality type, two Types in mixed case. Clean apart from the info line. | | `sensors-bad-type.yaml` | `humidity` is not a field name, so the file is refused and the valid types are listed. | | `sensors-not-a-list.yaml` | One sensor written as a mapping, not a one-item list: nothing would be read, so refused. | | `sensors-warnings.yaml` | Non-numeric and `.inf` `Scale` (both read as 1), an unknown key, one Type listed twice. Four warnings. | diff --git a/test/fixtures/portduino-config/sensors.yaml b/test/fixtures/portduino-config/sensors.yaml index 5517828d0b..5e8bb8e965 100644 --- a/test/fixtures/portduino-config/sensors.yaml +++ b/test/fixtures/portduino-config/sensors.yaml @@ -1,5 +1,5 @@ -# Two command sensors, one with a Scale and one with a Type in a different case. Clean: the only -# finding is the info line saying they need environment measurement enabled. +# Three command sensors: one with a Scale, one with a Type in a different case, and one air-quality +# type. Clean: the only finding is the info line saying which Telemetry setting each needs enabled. Lora: Module: sx1262 CS: 21 @@ -12,3 +12,5 @@ Sensors: Scale: 0.001 - Type: Relative_Humidity Command: /usr/local/bin/read-humidity + - Type: CO2 + Command: /usr/local/bin/read-co2 diff --git a/test/test_command_sensor/test_main.cpp b/test/test_command_sensor/test_main.cpp index e07e0c5548..743c6fd30f 100644 --- a/test/test_command_sensor/test_main.cpp +++ b/test/test_command_sensor/test_main.cpp @@ -1,7 +1,7 @@ // Tests for the config.yaml "Sensors:" feature on Linux (meshtasticd): // // - execCommand() / exec() in src/platform/portduino/PortduinoGlue.cpp -// - CommandSensorFields::{canonicalType, apply, parseValue} in +// - CommandSensorFields::{canonicalType, messageFor, apply, parseValue} in // src/modules/Telemetry/Sensor/CommandSensorFields.cpp // - CommandSensor (worker thread and getMetrics()) in src/modules/Telemetry/Sensor/CommandSensor.cpp // @@ -21,6 +21,11 @@ // apply() writes into nanopb fields of several widths. A value outside a narrow integer field // (soil_moisture is uint8_t) must clamp, not wrap: 300 wrapping to 44% moisture is a silent lie. // +// A Type belongs to exactly one telemetry message, EnvironmentMetrics or AirQualityMetrics, and each +// telemetry module owns one CommandSensor that runs only its own entries. An entry claimed by both would +// run its command twice and could land in the wrong message; one claimed by neither would be silently +// dropped after loadConfig() accepted it. +// // CommandSensor runs commands on a worker thread and getMetrics() only reads the latest results. // getMetrics() is called on the main loop, which also drives the radio, so it must return at once even // while a command is hanging. On Windows, where the timeout cannot be enforced, that is the only thing @@ -89,6 +94,14 @@ bool waitForReads(CommandSensor &sensor, uint32_t reads, long timeoutMs = 5000) return true; } +meshtastic_Telemetry emptyAirQuality() +{ + meshtastic_Telemetry t = meshtastic_Telemetry_init_zero; + t.which_variant = meshtastic_Telemetry_air_quality_metrics_tag; + t.variant.air_quality_metrics = meshtastic_AirQualityMetrics_init_zero; + return t; +} + meshtastic_Telemetry emptyEnvironment() { meshtastic_Telemetry t = meshtastic_Telemetry_init_zero; @@ -264,11 +277,38 @@ void test_apply_unknown_type_changes_nothing() TEST_ASSERT_EQUAL_MEMORY(&zero, &m, sizeof(m)); } +void test_messageFor_routes_each_type_to_one_message() +{ + using CommandSensorFields::Message; + TEST_ASSERT_TRUE(CommandSensorFields::messageFor("temperature") == Message::Environment); + TEST_ASSERT_TRUE(CommandSensorFields::messageFor("iaq") == Message::Environment); + TEST_ASSERT_TRUE(CommandSensorFields::messageFor("PM25_Standard") == Message::AirQuality); + TEST_ASSERT_TRUE(CommandSensorFields::messageFor("co2") == Message::AirQuality); + TEST_ASSERT_TRUE(CommandSensorFields::messageFor("humidity") == Message::None); + // A status bitmask, not a measurement: deliberately not a Type. + TEST_ASSERT_TRUE(CommandSensorFields::messageFor("pm_status_flags") == Message::None); + TEST_ASSERT_EQUAL_STRING("pm25_standard", CommandSensorFields::canonicalType("PM25_Standard").c_str()); +} + +void test_apply_air_quality_fields() +{ + meshtastic_AirQualityMetrics m = meshtastic_AirQualityMetrics_init_zero; + TEST_ASSERT_TRUE(CommandSensorFields::apply(m, "co2", 412.6f)); + TEST_ASSERT_TRUE(m.has_co2); + TEST_ASSERT_EQUAL_UINT32(413, m.co2); + TEST_ASSERT_TRUE(CommandSensorFields::apply(m, "pm25_standard", -3.0f)); + TEST_ASSERT_EQUAL_UINT32(0, m.pm25_standard); + TEST_ASSERT_TRUE(CommandSensorFields::apply(m, "pm_voc_idx", 101.5f)); + TEST_ASSERT_EQUAL_FLOAT(101.5f, m.pm_voc_idx); + // An environment type is not an air-quality field. + TEST_ASSERT_FALSE(CommandSensorFields::apply(m, "temperature", 20.0f)); +} + // --- CommandSensor --- void test_CommandSensor_not_added_without_config() { - CommandSensor sensor; + CommandSensor sensor(CommandSensorFields::Message::Environment); TEST_ASSERT_FALSE(sensor.initDevice(nullptr, nullptr)); } @@ -277,7 +317,7 @@ void test_CommandSensor_applies_scale_and_skips_bad_output() portduino_config.command_sensors.push_back({"temperature", "echo 21500", 0.001f}); portduino_config.command_sensors.push_back({"relative_humidity", "echo 'no sensor'", 1.0f}); portduino_config.command_sensors.push_back({"lux", "exit 1", 1.0f}); - CommandSensor sensor; + CommandSensor sensor(CommandSensorFields::Message::Environment); TEST_ASSERT_TRUE(sensor.initDevice(nullptr, nullptr)); TEST_ASSERT_TRUE(waitForReads(sensor, 3)); @@ -294,7 +334,7 @@ void test_CommandSensor_rejects_scale_overflow() // Both factors are finite but the product is not; it must not be sent, nor reach an integer cast. portduino_config.command_sensors.push_back({"temperature", "echo 3e38", 10.0f}); portduino_config.command_sensors.push_back({"soil_moisture", "echo 3e38", 10.0f}); - CommandSensor sensor; + CommandSensor sensor(CommandSensorFields::Message::Environment); TEST_ASSERT_TRUE(sensor.initDevice(nullptr, nullptr)); TEST_ASSERT_TRUE(waitForReads(sensor, 2)); meshtastic_Telemetry t = emptyEnvironment(); @@ -306,7 +346,7 @@ void test_CommandSensor_rejects_scale_overflow() void test_CommandSensor_all_failing_reports_no_metrics() { portduino_config.command_sensors.push_back({"temperature", "echo oops", 1.0f}); - CommandSensor sensor; + CommandSensor sensor(CommandSensorFields::Message::Environment); TEST_ASSERT_TRUE(sensor.initDevice(nullptr, nullptr)); TEST_ASSERT_TRUE(waitForReads(sensor, 1)); meshtastic_Telemetry t = emptyEnvironment(); @@ -320,7 +360,7 @@ void test_CommandSensor_caches_readings_then_refreshes() std::remove(runs.c_str()); const std::string cmd = "echo x >> " + runs + "; echo 7"; portduino_config.command_sensors.push_back({"weight", cmd, 1.0f}); - CommandSensor sensor; + CommandSensor sensor(CommandSensorFields::Message::Environment); TEST_ASSERT_TRUE(sensor.initDevice(nullptr, nullptr)); TEST_ASSERT_TRUE(waitForReads(sensor, 1)); @@ -343,7 +383,7 @@ void test_CommandSensor_getMetrics_does_not_wait_for_a_hung_command() { // The worker is stuck in this command for its full 2 s timeout; getMetrics() must not be. portduino_config.command_sensors.push_back({"lux", "sleep 10", 1.0f}); - CommandSensor sensor; + CommandSensor sensor(CommandSensorFields::Message::Environment); TEST_ASSERT_TRUE(sensor.initDevice(nullptr, nullptr)); usleep(200 * 1000); @@ -358,6 +398,44 @@ void test_CommandSensor_getMetrics_does_not_wait_for_a_hung_command() TEST_ASSERT_FALSE(sensor.getMetrics(&t)); } +void test_CommandSensor_each_module_runs_only_its_own_types() +{ + portduino_config.command_sensors.push_back({"temperature", "echo 21", 1.0f}); + portduino_config.command_sensors.push_back({"co2", "echo 415", 1.0f}); + CommandSensor environment(CommandSensorFields::Message::Environment); + CommandSensor airQuality(CommandSensorFields::Message::AirQuality); + TEST_ASSERT_TRUE(environment.initDevice(nullptr, nullptr)); + TEST_ASSERT_TRUE(airQuality.initDevice(nullptr, nullptr)); + TEST_ASSERT_TRUE(waitForReads(environment, 1)); + TEST_ASSERT_TRUE(waitForReads(airQuality, 1)); + usleep(300 * 1000); + // One command each: neither instance runs the other's entry. + TEST_ASSERT_EQUAL(1, environment.finishedReads()); + TEST_ASSERT_EQUAL(1, airQuality.finishedReads()); + + meshtastic_Telemetry env = emptyEnvironment(); + TEST_ASSERT_TRUE(environment.getMetrics(&env)); + TEST_ASSERT_TRUE(env.variant.environment_metrics.has_temperature); + + meshtastic_Telemetry aq = emptyAirQuality(); + TEST_ASSERT_TRUE(airQuality.getMetrics(&aq)); + TEST_ASSERT_TRUE(aq.variant.air_quality_metrics.has_co2); + TEST_ASSERT_EQUAL_UINT32(415, aq.variant.air_quality_metrics.co2); + + // Handed the other module's message, an instance writes nothing into it. + meshtastic_Telemetry wrong = emptyAirQuality(); + TEST_ASSERT_FALSE(environment.getMetrics(&wrong)); + meshtastic_AirQualityMetrics zero = meshtastic_AirQualityMetrics_init_zero; + TEST_ASSERT_EQUAL_MEMORY(&zero, &wrong.variant.air_quality_metrics, sizeof(zero)); +} + +void test_CommandSensor_not_added_without_its_module_types() +{ + portduino_config.command_sensors.push_back({"temperature", "echo 21", 1.0f}); + CommandSensor airQuality(CommandSensorFields::Message::AirQuality); + TEST_ASSERT_FALSE(airQuality.initDevice(nullptr, nullptr)); +} + void test_CommandSensor_destructor_stops_the_worker() { // Destroying the sensor must join its worker rather than leave a thread running a destroyed object. @@ -366,7 +444,7 @@ void test_CommandSensor_destructor_stops_the_worker() const std::string cmd = "echo x >> " + runs + "; echo 1"; portduino_config.command_sensors.push_back({"weight", cmd, 1.0f}); { - CommandSensor sensor; + CommandSensor sensor(CommandSensorFields::Message::Environment); TEST_ASSERT_TRUE(sensor.initDevice(nullptr, nullptr)); TEST_ASSERT_TRUE(waitForReads(sensor, 1)); } @@ -397,12 +475,16 @@ void setup() RUN_TEST(test_apply_sets_value_and_has_flag); RUN_TEST(test_apply_rounds_and_clamps_integer_fields); RUN_TEST(test_apply_unknown_type_changes_nothing); + RUN_TEST(test_messageFor_routes_each_type_to_one_message); + RUN_TEST(test_apply_air_quality_fields); RUN_TEST(test_CommandSensor_not_added_without_config); RUN_TEST(test_CommandSensor_applies_scale_and_skips_bad_output); RUN_TEST(test_CommandSensor_rejects_scale_overflow); RUN_TEST(test_CommandSensor_all_failing_reports_no_metrics); RUN_TEST(test_CommandSensor_caches_readings_then_refreshes); RUN_TEST(test_CommandSensor_getMetrics_does_not_wait_for_a_hung_command); + RUN_TEST(test_CommandSensor_each_module_runs_only_its_own_types); + RUN_TEST(test_CommandSensor_not_added_without_its_module_types); RUN_TEST(test_CommandSensor_destructor_stops_the_worker); const int failures = UNITY_END();