From fcef46f4b0591efa1e957a8df15c4c277521a4d7 Mon Sep 17 00:00:00 2001 From: Tom <116762865+NomDeTom@users.noreply.github.com> Date: Tue, 5 May 2026 12:52:07 +0100 Subject: [PATCH] dependency swap - INA3221Sensor (#10379) * dependency swap - INA3221Sensor update INA3221 initialization and measurement methods for compatibility with Rob Tillaart's library Co-authored-by: Copilot * Addresses copilot review Co-authored-by: Copilot * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Refine comments on USB detection and INA3221 Updated comments regarding USB detection and INA3221 usage. * Fix static_assert conditions for INA3221 channel definitions Co-authored-by: Copilot * moved macro defines earlier to allow better use. Co-authored-by: Copilot * Add raw register read methods for bus voltage and shunt current in INA3221Sensor Co-authored-by: Copilot --------- Co-authored-by: Copilot Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- platformio.ini | 4 +- .../Telemetry/Sensor/INA3221Sensor.cpp | 67 ++++++++++++++----- src/modules/Telemetry/Sensor/INA3221Sensor.h | 25 +++++-- variants/nrf52840/rak3401_1watt/variant.h | 4 +- variants/nrf52840/rak4631/variant.h | 4 +- 5 files changed, 79 insertions(+), 25 deletions(-) diff --git a/platformio.ini b/platformio.ini index 8a3129075..95c89a3ef 100644 --- a/platformio.ini +++ b/platformio.ini @@ -170,8 +170,8 @@ lib_deps = https://github.com/EmotiBit/EmotiBit_MLX90632/archive/refs/tags/v1.0.8.zip # renovate: datasource=github-tags depName=Adafruit MLX90614 packageName=adafruit/Adafruit_MLX90614 https://github.com/adafruit/Adafruit-MLX90614-Library/archive/refs/tags/2.1.6.zip - # renovate: datasource=git-refs depName=INA3221 packageName=https://github.com/sgtwilko/INA3221 gitBranch=FixOverflow - https://github.com/sgtwilko/INA3221/archive/bb03d7e9bfcc74fc798838a54f4f99738f29fc6a.zip + # renovate: datasource=github-tags depName=INA3221_RT packageName=RobTillaart/INA3221_RT + https://github.com/RobTillaart/INA3221_RT/archive/refs/tags/0.4.2.zip # renovate: datasource=github-tags depName=QMC5883L Compass packageName=mprograms/QMC5883LCompass https://github.com/mprograms/QMC5883LCompass/archive/refs/tags/v1.2.3.zip # renovate: datasource=github-tags depName=DFRobot_RTU packageName=dfrobot/DFRobot_RTU diff --git a/src/modules/Telemetry/Sensor/INA3221Sensor.cpp b/src/modules/Telemetry/Sensor/INA3221Sensor.cpp index 78081132a..d3b9b16f0 100644 --- a/src/modules/Telemetry/Sensor/INA3221Sensor.cpp +++ b/src/modules/Telemetry/Sensor/INA3221Sensor.cpp @@ -16,10 +16,28 @@ int32_t INA3221Sensor::runOnce() return DEFAULT_SENSOR_MINIMUM_WAIT_TIME_BETWEEN_READS; } if (!status) { - ina3221.begin(nodeTelemetrySensorsMap[sensorType].second); - ina3221.setShuntRes(100, 100, 100); // 0.1 Ohm shunt resistors - status = true; + // Re-initialise with the address and Wire bus from the telemetry sensors map. + // (Rob Tillaart INA3221_RT takes address + TwoWire*, unlike sgtwilko which took Wire in begin().) + ina3221 = INA3221(nodeTelemetrySensorsMap[sensorType].first, nodeTelemetrySensorsMap[sensorType].second); + status = ina3221.begin(); + if (status) { + // Default all three channels to a 0.1 Ω shunt resistor. + // Override per-variant by defining INA3221_SHUNT_R_CH1/CH2/CH3 (in Ohms) in variant.h. +#ifndef INA3221_SHUNT_R_CH1 +#define INA3221_SHUNT_R_CH1 0.1f +#endif +#ifndef INA3221_SHUNT_R_CH2 +#define INA3221_SHUNT_R_CH2 0.1f +#endif +#ifndef INA3221_SHUNT_R_CH3 +#define INA3221_SHUNT_R_CH3 0.1f +#endif + ina3221.setShuntR(0, INA3221_SHUNT_R_CH1); + ina3221.setShuntR(1, INA3221_SHUNT_R_CH2); + ina3221.setShuntR(2, INA3221_SHUNT_R_CH3); + } } else { + // Already initialised; status stays true and initI2CSensor() returns next poll interval. status = true; } return initI2CSensor(); @@ -27,12 +45,14 @@ int32_t INA3221Sensor::runOnce() void INA3221Sensor::setup() {} -struct _INA3221Measurement INA3221Sensor::getMeasurement(ina3221_ch_t ch) +struct _INA3221Measurement INA3221Sensor::getMeasurement(uint8_t ch) { struct _INA3221Measurement measurement; - measurement.voltage = ina3221.getVoltage(ch); - measurement.current = ina3221.getCurrent(ch); + measurement.voltage = ina3221.getBusVoltage(ch); // Volts + // getCurrent_mA() is used instead of getCurrent() because Rob Tillaart's getCurrent() + // returns Amperes; the telemetry proto and VoltageSensor/CurrentSensor interfaces expect mA. + measurement.current = ina3221.getCurrent_mA(ch); // milliAmps return measurement; } @@ -43,7 +63,7 @@ struct _INA3221Measurements INA3221Sensor::getMeasurements() // INA3221 has 3 channels starting from 0 for (int i = 0; i < 3; i++) { - measurements.measurements[i] = getMeasurement((ina3221_ch_t)i); + measurements.measurements[i] = getMeasurement((uint8_t)i); } return measurements; @@ -87,24 +107,41 @@ bool INA3221Sensor::getPowerMetrics(meshtastic_Telemetry *measurement) measurement->variant.power_metrics.has_ch3_voltage = true; measurement->variant.power_metrics.has_ch3_current = true; - measurement->variant.power_metrics.ch1_voltage = m.measurements[INA3221_CH1].voltage; - measurement->variant.power_metrics.ch1_current = m.measurements[INA3221_CH1].current; - measurement->variant.power_metrics.ch2_voltage = m.measurements[INA3221_CH2].voltage; - measurement->variant.power_metrics.ch2_current = m.measurements[INA3221_CH2].current; - measurement->variant.power_metrics.ch3_voltage = m.measurements[INA3221_CH3].voltage; - measurement->variant.power_metrics.ch3_current = m.measurements[INA3221_CH3].current; + // INA3221 channel indices are zero-based (0=CH1, 1=CH2, 2=CH3). + measurement->variant.power_metrics.ch1_voltage = m.measurements[0].voltage; + measurement->variant.power_metrics.ch1_current = m.measurements[0].current; + measurement->variant.power_metrics.ch2_voltage = m.measurements[1].voltage; + measurement->variant.power_metrics.ch2_current = m.measurements[1].current; + measurement->variant.power_metrics.ch3_voltage = m.measurements[2].voltage; + measurement->variant.power_metrics.ch3_current = m.measurements[2].current; return true; } uint16_t INA3221Sensor::getBusVoltageMv() { - return lround(ina3221.getVoltage(BAT_CH) * 1000); + return lround(ina3221.getBusVoltage_mV(BAT_CH)); } int16_t INA3221Sensor::getCurrentMa() { - return lround(ina3221.getCurrent(BAT_CH)); + return lround(ina3221.getCurrent_mA(BAT_CH)); +} + +// Bus voltage register (0x02 + ch*2): bits [15:3] unsigned, 1 LSB = 8 mV (datasheet p.6). +// Voltage raw units: 1 count = 8 mV, so V_mV = raw * 8. +int16_t INA3221Sensor::getRawBusVoltage(uint8_t ch) +{ + return (int16_t)(ina3221.getRegister(0x02 + ch * 2) >> 3); +} + +// Shunt voltage register (0x01 + ch*2): bits [15:3] signed two's complement, 1 LSB = 40 µV (datasheet p.6). +// Current raw units are shunt-voltage counts: 1 count = 40 uV, signed. +// I_mA = (raw * 40 uV) / R_mOhm, because uV / mOhm = mA. +// Example for 100 mOhm shunt: I_mA = raw * 40 / 100 = raw * 0.4. +int16_t INA3221Sensor::getRawShuntCurrent(uint8_t ch) +{ + return (int16_t)(ina3221.getRegister(0x01 + ch * 2) >> 3); } #endif \ No newline at end of file diff --git a/src/modules/Telemetry/Sensor/INA3221Sensor.h b/src/modules/Telemetry/Sensor/INA3221Sensor.h index 0581f92f6..bb2dbe7b3 100644 --- a/src/modules/Telemetry/Sensor/INA3221Sensor.h +++ b/src/modules/Telemetry/Sensor/INA3221Sensor.h @@ -1,3 +1,9 @@ +// INA3221 channel aliases (zero-based: 0 = CH1, 1 = CH2, 2 = CH3). +// Defined before configuration.h so variant.h can use them in INA3221_ENV_CH / INA3221_BAT_CH. +#define INA3221_CH1 0 +#define INA3221_CH2 1 +#define INA3221_CH3 2 + #include "configuration.h" #if HAS_TELEMETRY && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR && __has_include() @@ -9,26 +15,29 @@ #include #ifndef INA3221_ENV_CH -#define INA3221_ENV_CH INA3221_CH1 +#define INA3221_ENV_CH INA3221_CH1 // channel to report in environment metrics (default: CH1) #endif #ifndef INA3221_BAT_CH -#define INA3221_BAT_CH INA3221_CH1 +#define INA3221_BAT_CH INA3221_CH1 // channel for device_battery_ina_address (default: CH1) #endif class INA3221Sensor : public TelemetrySensor, VoltageSensor, CurrentSensor { private: - INA3221 ina3221 = INA3221(INA3221_ADDR42_SDA); + // Placeholder constructor; re-initialised with correct address and Wire in runOnce(). + INA3221 ina3221 = INA3221(INA3221_ADDR); // channel to report voltage/current for environment metrics - static const ina3221_ch_t ENV_CH = INA3221_ENV_CH; + static const uint8_t ENV_CH = INA3221_ENV_CH; + static_assert(INA3221_ENV_CH >= 0 && INA3221_ENV_CH <= 2, "INA3221_ENV_CH must be 0, 1, or 2"); // channel to report battery voltage for device_battery_ina_address - static const ina3221_ch_t BAT_CH = INA3221_BAT_CH; + static const uint8_t BAT_CH = INA3221_BAT_CH; + static_assert(INA3221_BAT_CH >= 0 && INA3221_BAT_CH <= 2, "INA3221_BAT_CH must be 0, 1, or 2"); // get a single measurement for a channel - struct _INA3221Measurement getMeasurement(ina3221_ch_t ch); + struct _INA3221Measurement getMeasurement(uint8_t ch); // get all measurements for all channels struct _INA3221Measurements getMeasurements(); @@ -45,6 +54,10 @@ class INA3221Sensor : public TelemetrySensor, VoltageSensor, CurrentSensor bool getMetrics(meshtastic_Telemetry *measurement) override; virtual uint16_t getBusVoltageMv() override; virtual int16_t getCurrentMa() override; + + // Raw register reads (bits [15:3] right-shifted), no conversion applied. + int16_t getRawBusVoltage(uint8_t ch); + int16_t getRawShuntCurrent(uint8_t ch); }; struct _INA3221Measurement { diff --git a/variants/nrf52840/rak3401_1watt/variant.h b/variants/nrf52840/rak3401_1watt/variant.h index 80b09cf69..a154e6a41 100644 --- a/variants/nrf52840/rak3401_1watt/variant.h +++ b/variants/nrf52840/rak3401_1watt/variant.h @@ -166,7 +166,9 @@ static const uint8_t SCK = PIN_SPI_SCK; // Testing USB detection #define NRF_APM // If using a power chip like the INA3221 you can override the default battery voltage channel below -// and comment out NRF_APM to use the INA3221 instead of the USB detection for charging +// and comment out NRF_APM to use the INA3221 instead of the USB detection for charging. +// INA3221Sensor.h provides INA3221_CH1/INA3221_CH2/INA3221_CH3 compatibility aliases, so +// board variants can continue to use the named channel constants here. // #define INA3221_BAT_CH INA3221_CH2 // #define INA3221_ENV_CH INA3221_CH1 diff --git a/variants/nrf52840/rak4631/variant.h b/variants/nrf52840/rak4631/variant.h index 6a6b32f27..8ea272a15 100644 --- a/variants/nrf52840/rak4631/variant.h +++ b/variants/nrf52840/rak4631/variant.h @@ -220,7 +220,9 @@ SO GPIO 39/TXEN MAY NOT BE DEFINED FOR SUCCESSFUL OPERATION OF THE SX1262 - TG // Testing USB detection #define NRF_APM // If using a power chip like the INA3221 you can override the default battery voltage channel below -// and comment out NRF_APM to use the INA3221 instead of the USB detection for charging +// and comment out NRF_APM to use the INA3221 instead of the USB detection for charging. +// INA3221Sensor.h provides compatibility aliases such as INA3221_CH1/INA3221_CH2/INA3221_CH3, +// so board variants can continue to use the channel names below. // #define INA3221_BAT_CH INA3221_CH2 // #define INA3221_ENV_CH INA3221_CH1