Skip to content

Integration of ADS1X15 ADC#7109

Closed
oscgonfer wants to merge 6 commits intomeshtastic:masterfrom
fablabbcn:enhancement/ads1x15-integration
Closed

Integration of ADS1X15 ADC#7109
oscgonfer wants to merge 6 commits intomeshtastic:masterfrom
fablabbcn:enhancement/ads1x15-integration

Conversation

@oscgonfer
Copy link
Contributor

@oscgonfer oscgonfer commented Jun 23, 2025

This is a WIP Pull request for the TI ADS1X15 ADCs, discussed in our fork.

I am submitting a separate PR for the protobuf adataptations that need to happen to accomodate for all the channels, but needs discussion (as generally we are interested in having 8 channel, or two ADCs in the same I2C bus), using this board: https://docs.smartcitizen.me/hardware/boards/analog-sensor-board/

Currently this is not working as expected.

I am able to detect and identify the ADS1X15 (note that the default address coincides with another device):

INFO  | ??:??:?? 1 SHT31 found at address 0x44
INFO  | ??:??:?? 1 ADS1X15 found at address 0x48
INFO  | ??:??:?? 1 ADS1X15 found at address 0x49

EDIT:
However, when doing the scan in the setup(): e21d72c#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R693 I don't seem to be able to add the ADC(s) to the sensor list.

EDIT:
Initially, I thought this was because there is only one sensorType and adding two entries to the nodeTelemetrySensorsMap creates a conflict: https://github.com/fablabbcn/smartcitizen-meshtastic/blob/e21d72c719c5f6d052bd72394481f7f758dee4b8/src/main.cpp#L1471-L1476. I think this can be a problem (specially with daisy chained sensors). There could be a potential change on the mapping as per this issue

However, I have also tested a single ADC, and despite detecting it correctly, I don't seem to be adding them correctly to the nodeTelemetrySensorsMap. Then, once we get here https://github.com/fablabbcn/smartcitizen-meshtastic/blob/e21d72c719c5f6d052bd72394481f7f758dee4b8/src/modules/Telemetry/PowerTelemetry.cpp#L84, its not found.

Maybe another pair of eyes can take a look and see where I am making my mistake.

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Seeed Xiao ESP32-S3

@oscgonfer
Copy link
Contributor Author

Goes with meshtastic/protobufs#711

@oscgonfer
Copy link
Contributor Author

Progress: now this is working, BUT, there is a small issue that I am not sure how to go around:

the serial log shows data is being read in all four channels (ADS1X15 has 4 channels)

INFO  | 09:00:54 347 [PowerTelemetry] Checking sensor type addr 0x48
INFO  | 09:00:54 347 [PowerTelemetry] Getting ADS1X15 sensor
INFO  | 09:00:55 348 [PowerTelemetry] Send: ch1_voltage=0.314844, ch1_current=0.000000, ch2_voltage=3.336625, ch2_current=0.000000, ch3_voltage=0.318375, ch3_current=0.000000, ch4_voltage=0.319000

BUT I don't see all four channels in the received protobuf on our MQTT broker:

decoded {
  portnum: TELEMETRY_APP
  payload: "time: 1750842420 power_metrics {   ch1_voltage: 0.315375   ch2_voltage: 3.33675   ch3_voltage: 0.318312496 }"
}

I think this should be the case, but likely I am missing something silly. Probably the core team can give a hand?

@caveman99
Copy link
Member

caveman99 commented Jun 25, 2025

For a quick fix to use both sensors at the same time you could define them as two different labels, ADS1X15 and ADS1X15_ALT and write your module logic in a way it queries both sensors if available. The propler fix would be the change to the sensor map though :-)

--EDIT-- disregard, that would require another mapping for the sensor in the protobufs, better go with the reshuffle.

@oscgonfer oscgonfer marked this pull request as ready for review June 25, 2025 09:56
@oscgonfer
Copy link
Contributor Author

@caveman99 I am having second thoughts regarding this PR... As it feels that it creates a bit of "bloat" on the powermetrics module, for just one chip. Maybe there is another place to put these voltages (something like AnalogTelemetry?)

@caveman99
Copy link
Member

caveman99 commented Jun 25, 2025

I'd put it here. All the values are optional so are not taking up any space if not used.and an 8 channel adc is not an unusual use case ;-)

@oscgonfer
Copy link
Contributor Author

An additional comment, I had initially inherited the ADS1X15Sensor class from both TelemetrySensor and VoltageSensor to later remove the VoltageSensor one. I am not sure if there is any logic behind that choice, but worth noting that.

@oscgonfer
Copy link
Contributor Author

oscgonfer commented Jun 28, 2025

Just added an additional type to support two sensors in parallel. Without the reshuffle I don't see another option of doing this. This basically creates a new type, and modifies ADS1X15Sensor class to also take the sensorType as an argument... This then allows to create two ADS1X15Sensor objects, but each with a different type. The class internally then maps the readings to one or the other, which is really not very neat, but to fix later after the reshuffle.

6eef108

This also needs https://github.com/fablabbcn/protobufs/tree/enhancement/additional-ads1x15-support to work in the protobufs submodule. PR here: #7109

@oscgonfer oscgonfer force-pushed the enhancement/ads1x15-integration branch from 6eef108 to bb828a4 Compare June 30, 2025 16:46
@oscgonfer oscgonfer changed the title Initial integration of ADS1X15 ADC Integration of ADS1X15 ADC Jul 1, 2025
@oscgonfer
Copy link
Contributor Author

oscgonfer commented Jul 1, 2025

I think this also requires a bit more of testing with two ADCs. Not sure what the issue is. With one, I consistently receive data from it at the right values. I have also used two daisy chained ones at 3V3 (I2C and power) with this breakout from adafruit:

decoded { portnum: TELEMETRY_APP payload: "time: 1751371129 power_metrics { ch1_voltage: 0.32134375 ch2_voltage: 0.339187503 ch3_voltage: 0.3255 ch4_voltage: 3.33587503 ch5_voltage: 0.3105 ch6_voltage: 0.32490626 ch7_voltage: 0.314156264 ch8_voltage: 3.33562493 }" }

However, when using our own custom board (design here), which uses a step-up to work at 5V with level shifters, I get this:

decoded {
  portnum: TELEMETRY_APP
  payload: "time: 1751368488 power_metrics {   ch1_voltage: 0.0438125   ch2_voltage: 0.00796875   ch3_voltage: 0.111968748   ch4_voltage: 2.04796886   ch5_voltage: 12.2878122   ch6_voltage: 12.2878122   ch7_voltage: 12.2878122   ch8_voltage: 12.2878122 }"
}

The 12.28V values are due to an overflow on the variables (when too close to 0V), and it also happens on our end. This can ultimately be very easily filtered (and simply considered as 0V). However, I think there is

Besides the obvious test with another board, I have used this with our own hardware and it seems fine, so it's probably noise in the I2C lines on the Xiao because of the step up... ?

@oscgonfer oscgonfer force-pushed the enhancement/ads1x15-integration branch from bb828a4 to 7962f51 Compare July 3, 2025 08:01
@oscgonfer
Copy link
Contributor Author

Rebased onto master

@oscgonfer oscgonfer closed this Jul 12, 2025
@oscgonfer oscgonfer deleted the enhancement/ads1x15-integration branch July 12, 2025 12:36
@oscgonfer
Copy link
Contributor Author

Errrm... how did this get closed? 😓

@oscgonfer
Copy link
Contributor Author

Moved to #7404 as I can't reopen now...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants