Skip to content

Conversation

@oscgonfer
Copy link
Contributor

@oscgonfer oscgonfer commented Jul 1, 2025

This Pull request aims to have a clean AirQualityTelemetry module, decoupled from the PMSA003I sensor integration, and update to the latest changes in master.

It already contains the work by @fifieldt on #7187. Other efforts have been done as part of other sensor integrations such as this #4601 for the CO2 sensor SCD4X (in this commit 318da22) or in this other https://github.com/hafu/meshtastic-firmware/tree/add-scd30 to add a SCD30 (in this commit 735b428).

I open a separate one to be able to test this features separately, and create other branches separate to integrate those sensors (and others as part of the hackathon). The features to test are:

  • Check this is the right implementation of the readings for this particular PMS sensor
  • Check the logic to send to sleep and wake up the sensor. This is not added in other PRs and it's critical for sensors that have such high consumption as the PM sensors, which draw +200mA or more on boot
  • Check the rescan logic originally implemented as part of the AirQualityTelemetry class, I assume for those sensors that have a long boot time
  • Check that this all works with the recent screen changes

Sadly, I don't have a PMSA003I sensor with me at the moment (I have ordered one), but if anyone has one, it'd be great if you can check and/or give feedback. I think the original implementer @thebentern and @fifieldt would have a lot to say. I'll try to be responsive to be able to fix.

NOTE: Since I am making a PR from a separate org github doesn't allow for edits (see this issue).

@oscgonfer oscgonfer mentioned this pull request Jul 2, 2025
3 tasks
@oscgonfer oscgonfer marked this pull request as ready for review July 2, 2025 10:08
@thebentern thebentern requested a review from Copilot July 2, 2025 10:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR decouples the PMSA003I sensor from the AirQualityTelemetryModule into its own class and brings the telemetry code up to date with master.

  • Extracted PMSA003I logic into PMSA003ISensor
  • Refactored AirQualityTelemetryModule to delegate sensor operations and add deep‐sleep support
  • Cleaned up legacy fields and removed direct sensor handling from environmental telemetry

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/modules/Telemetry/Sensor/PMSA003ISensor.h New sensor class declaration and warmup constant
src/modules/Telemetry/Sensor/PMSA003ISensor.cpp New sensor class implementation and I2C scanning logic
src/modules/Telemetry/EnvironmentTelemetry.h Removed unused sensor_read_error_count field
src/modules/Telemetry/EnvironmentTelemetry.cpp Removed reset of sensor_read_error_count
src/modules/Telemetry/AirQualityTelemetry.h Updated module to use PMSA003ISensor, UI stubs, and config
src/modules/Telemetry/AirQualityTelemetry.cpp Refactored run/send logic, deep‐sleep integration, UI drawing
src/modules/Modules.cpp Always register AirQualityTelemetryModule
Comments suppressed due to low confidence (7)

src/modules/Telemetry/Sensor/PMSA003ISensor.h:41

  • The call to pinMode is placed at class scope, which is invalid C++; move it into the constructor or setup() method.
    pinMode(PMSA003I_ENABLE_PIN, OUTPUT);

src/modules/Telemetry/Sensor/PMSA003ISensor.cpp:54

  • Definition of sleep() is missing the class scope qualifier; it should be defined as void PMSA003ISensor::sleep().
void sleep() {

src/modules/Telemetry/Sensor/PMSA003ISensor.cpp:59

  • Definition of wakeUp() is missing the class scope qualifier and lacks a return statement; it should be uint32_t PMSA003ISensor::wakeUp() and return the warmup delay.
uint32_t wakeUp() {

src/modules/Telemetry/Sensor/PMSA003ISensor.cpp:68

  • Typo in the log message: "AQIn" should be "AQI".
        LOG_WARN("Skip send measurements. Could not read AQIn");

src/modules/Telemetry/AirQualityTelemetry.cpp:29

  • sleepOnNextExecution is used but not declared in AirQualityTelemetryModule; add a bool sleepOnNextExecution member.
    if (sleepOnNextExecution == true) {

src/modules/Telemetry/AirQualityTelemetry.cpp:77

  • state is a private member of PMSA003ISensor and pmsa003iSensor::State::IDLE is invalid syntax; either expose a getter or use PMSA003ISensor::State::IDLE and adjust access.
        if (pmsa003iSensor.hasSensor() && pmsa003iSensor.state == pmsa003iSensor::State::IDLE)

src/modules/Modules.cpp:218

  • [nitpick] Instantiating AirQualityTelemetryModule unconditionally may start the module without a sensor; consider guarding this with a sensor‐presence check.
        new AirQualityTelemetryModule();

@oscgonfer
Copy link
Contributor Author

@thebentern I have fixed (I think) comments by copilot #7190 (review)

@oscgonfer oscgonfer force-pushed the enhancement/air-quality-module branch 2 times, most recently from 660242e to b9ab6ef Compare July 3, 2025 07:56
@oscgonfer
Copy link
Contributor Author

Rebased on to master

Copy link
Contributor

@hafu hafu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw your PR and checked out, while compiling for different targets I came across some issues.

@oscgonfer oscgonfer force-pushed the enhancement/air-quality-module branch 2 times, most recently from 0d60d0e to c545a9f Compare July 6, 2025 06:34
@oscgonfer
Copy link
Contributor Author

Merged master and resolved @hafu comments.

@caveman99 caveman99 force-pushed the enhancement/air-quality-module branch from b0436b3 to 1f8b510 Compare July 14, 2025 13:54
@CLAassistant
Copy link

CLAassistant commented Jul 22, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ oscgonfer
❌ peterzqx
You have signed the CLA already but the status is still pending? Let us recheck it.

@Nashui-Yan Nashui-Yan force-pushed the enhancement/air-quality-module branch from 854a562 to 7686d46 Compare July 23, 2025 10:46
@oscgonfer oscgonfer force-pushed the enhancement/air-quality-module branch from f11f60c to 1b28fc1 Compare July 24, 2025 10:45
@github-actions github-actions bot closed this Nov 7, 2025
@thebentern thebentern reopened this Nov 7, 2025
@thebentern thebentern added triaged Reviewed by the team, has enough information and ready to work on now. and removed Stale Issues that will be closed if not triaged. labels Nov 7, 2025
* Reworks AQ telemetry to match new dynamic allocation method
* Adds VBLE_I2C_CLOCK_SPEED build flag for sensors with different I2C speed requirements
* Reworks PMSA003I
@oscgonfer
Copy link
Contributor Author

oscgonfer commented Jan 8, 2026

Reworked based on https://github.com/meshtastic/firmware/pull/8054/changes and tested with seeed-xiao-s3.

Also adds CAN_RECLOCK_I2C to build flags for esp32_common architecture as it's the only one that read and change I2C speed on demand, which many sensors require and that are not compatible with higher I2C bus speeds needed by the oled display. This last aspect is added on the TelemetrySensor class, with a logic to check on each architecture.

It needs meshtastic/protobufs#840

@oscgonfer
Copy link
Contributor Author

oscgonfer commented Jan 12, 2026

@thebentern
Copy link
Contributor

Not sure where this part should go. Maybe in src/detect?

https://github.com/fablabbcn/smartcitizen-meshtastic/blob/0d420ebc26f526b2e06e8f77abd8b8e42e4c429f/src/modules/Telemetry/Sensor/TelemetrySensor.cpp#L10-L44

Sounds good to me

@oscgonfer
Copy link
Contributor Author

oscgonfer commented Jan 14, 2026

All builds pass? Is this real life? 👀

@thebentern thebentern merged commit 5a81403 into meshtastic:master Jan 14, 2026
15 checks passed
@oscgonfer oscgonfer deleted the enhancement/air-quality-module branch January 14, 2026 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request triaged Reviewed by the team, has enough information and ready to work on now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants