Mirrors_Framework/PX4-Autopilot
1
0
Fork 0
mirror of https://github.com/PX4/PX4-Autopilot.git synced 2026-05-22 22:32:11 +08:00
Code Issues Actions 7 Packages Projects Releases Wiki Activity

refactor(ina238): rewrite driver for robustness and clarity

Browse Source 
Address customer-reported dropouts traced to several issues in the
prior driver:

- A single transient I2C blip flipped _initialized=false and re-ran
  probe()+Reset() inline on the next tick, losing most of a sample
  period on every isolated glitch.
- start() scheduled the first collect() ~100 ms after the device reset
  (INA238_SAMPLE_INTERVAL_US - 7 us), but the first averaged conversion
  takes 540 us x 3 channels x 64 samples ~= 103.7 ms -- so the first
  read sometimes hit post-reset zeros.
- The CONFIG register watchdog used set_bits/clear_bits that both
  collapsed to 0 in HIGH ADC range, making it a no-op for the default
  range. (ADCCONFIG and SHUNT_CAL were still checked.)
- setConnected(false) could fire up to three times per failed cycle
  (collect, register-check branch, RunImpl else), collapsing the
  intended ~2 s debounce.
- -t (battery index) was parsed with no range check; out-of-range
  values silently clamped to 1 inside Battery.

Rewrite:

- Non-blocking UNINITIALIZED / RESET / CONFIGURE / MEASURE state
  machine. Each step is its own RunImpl tick with an explicit
  ScheduleDelayed; init failures retry without losing the driver
  instance.
- Tolerate ~2 s of consecutive collect() failures
  (MAX_CONSECUTIVE_FAILURES = DISCONNECT_DEBOUNCE_US /
  SAMPLE_INTERVAL_US) before triggering a full reinit; isolated
  glitches just skip a cycle.
- SAMPLE_INTERVAL_US = 105 ms, just above the 103.68 ms ADC conversion
  period so we stop polling faster than the device produces samples.
  MEASURE compensates for in-tick I2C time so each tick is exactly
  SAMPLE_INTERVAL_US apart.
- CONFIGURE waits SAMPLE_INTERVAL_US + 5 ms before the first MEASURE
  read, comfortably above the first averaged conversion.
- checkConfigurationRotating() reads one config register per cycle and
  compares it against the value actually written, so an externally
  reset device is detected within three cycles regardless of ADC range.
- -t arg validated against 1-3 at parse time; out-of-range values now
  exit with an error.
- New perf counters ina238_bad_register and ina238_reinit. Driver
  state is also surfaced in 'ina238 status'.
- File layout: ina238_registers.hpp folded into ina238.h,
  ina238_main.cpp folded into ina238.cpp. Constants and enums namespaced
  under ina238. Params switched to DEFINE_PARAMETERS.
- RESET state publishes documented invalid sentinels (current = -1,
  temperature = NaN) instead of valid-looking zeros.

Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>

fix(ina238): pass literal i2c address to usage macro

PRINT_MODULE_USAGE_PARAMS_I2C_ADDRESS feeds its argument through the
module documentation parser's int(eval(...)) path, which only resolves
Python literals. The constexpr BASEADDR identifier crashed
make module_documentation with NameError. Match the convention in
sibling power_monitor drivers (ina220/ina226/ina228) and pass 0x45.

Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
This commit is contained in:
Jacob Dahl
2026-05-17 16:11:59 -06:00
parent 7f32786a29
commit 85e3cb88a7
5 changed files with 406 additions and 607 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/drivers/power_monitor/ina238/CMakeLists.txt
+1 -4
View File
@@ -1,6 +1,6 @@
############################################################################
#
# Copyright (c) 2021 PX4 Development Team. All rights reserved.
# Copyright (c) 2026 PX4 Development Team. All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions
@@ -33,10 +33,7 @@
px4_add_module(
MODULE drivers__ina238
MAIN ina238
COMPILE_FLAGS
-Wno-cast-align # TODO: fix and enable
SRCS
ina238_main.cpp
ina238.cpp
MODULE_CONFIG
ina238_params.yaml
src/drivers/power_monitor/ina238/ina238.cpp
+301 -268
View File
File diff suppressed because it is too large Load Diff
src/drivers/power_monitor/ina238/ina238.h
+104 -114
View File
@@ -1,6 +1,6 @@
/****************************************************************************
*
* Copyright (C) 2021 PX4 Development Team. All rights reserved.
* Copyright (c) 2026 PX4 Development Team. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -31,154 +31,144 @@
*
****************************************************************************/
#pragma once
#include <px4_platform_common/px4_config.h>
#include <px4_platform_common/getopt.h>
#include <drivers/device/i2c.h>
#include <lib/perf/perf_counter.h>
#include <battery/battery.h>
#include <drivers/drv_hrt.h>
#include <uORB/SubscriptionInterval.hpp>
#include <uORB/topics/parameter_update.h>
#include <lib/battery/battery.h>
#include <lib/perf/perf_counter.h>
#include <px4_platform_common/i2c_spi_buses.h>
#include "ina238_registers.hpp"
#include <px4_platform_common/module_params.h>
#include <px4_platform_common/px4_config.h>
using namespace time_literals;
using namespace ina238;
/* Configuration Constants */
#define INA238_BASEADDR 0x45 /* 7-bit address. 8-bit address is 0x45 */
// If initialization is forced (with the -f flag on the command line), but it fails, the drive will try again to
// connect to the INA238 every this many microseconds
#define INA238_INIT_RETRY_INTERVAL_US 500000
namespace ina238
{
static constexpr uint32_t BUS_CLOCK_HZ = 100'000;
#define INA238_MFG_ID_TI (0x5449) // TI
#define INA238_MFG_DIE (0x238) // INA237, INA238
static constexpr uint16_t MANFID = 0x5449;
static constexpr uint16_t DIEID = 0x238;
#define INA238_ADCRANGE_SHIFTS (4)
#define INA238_ADCRANGE_LOW (1 << INA238_ADCRANGE_SHIFTS) // ± 40.96 mV
#define INA238_ADCRANGE_HIGH (0 << INA238_ADCRANGE_SHIFTS) // ±163.84 mV
// Measurement scaling (from datasheet)
static constexpr float V_LSB = 3.125e-3f; // V per LSB
static constexpr float T_LSB = 7.8125e-3f; // °C per LSB
static constexpr float SHUNT_CAL_K = 819.2e6f; // shunt-cal scaling constant
static constexpr float ADCRANGE_LOW_V_SENSE = 0.04096f; // ±40.96 mV
#define INA238_DEVICE_ID_SHIFTS (4)
#define INA238_DEVICE_ID_MASK (0xfff << INA238_DEVICE_ID_SHIFTS)
#define INA238_DEVICEID(v) (((v) & INA238_DEVICE_ID_MASK) >> INA238_DEVICE_ID_SHIFTS)
// Sample timing
// ADC produces one averaged sample every 540us x 3 channels x 64 avg = 103.68 ms.
// Poll a hair slower than that so we always read a fresh sample rather than aliasing.
static constexpr hrt_abstime SAMPLE_INTERVAL_US = 105_ms;
#define INA238_SAMPLE_FREQUENCY_HZ 10
#define INA238_SAMPLE_INTERVAL_US (1_s / INA238_SAMPLE_FREQUENCY_HZ)
#define INA238_CONVERSION_INTERVAL (INA238_SAMPLE_INTERVAL_US - 7)
#define INA238_DN_MAX 32768.0f /* 2^15 */
#define INA238_CONST 819.2e6f /* is an internal fixed value used to ensure scaling is maintained properly */
#define INA238_VSCALE 3.125e-03f /* LSB of voltage is 3.1255 mV/LSB */
#define INA238_TSCALE 7.8125e-03f /* LSB of temperature is 7.8125 mDegC/LSB */
// Recovery / robustness timing
static constexpr hrt_abstime INIT_RETRY_INTERVAL_US = 500_ms;
static constexpr hrt_abstime RESET_DELAY_US = 1_ms; // datasheet specifies 300us. Give some margin
static constexpr hrt_abstime DISCONNECT_DEBOUNCE_US = 2_s;
static constexpr uint8_t MAX_CONSECUTIVE_FAILURES = DISCONNECT_DEBOUNCE_US / SAMPLE_INTERVAL_US;
#define INA238_ADCRANGE_LOW_V_SENSE 0.04096f // ± 40.96 mV
// Register map (subset used by this driver)
enum class Register : uint8_t {
CONFIG = 0x00,
ADCCONFIG = 0x01,
SHUNT_CAL = 0x02,
VS_BUS = 0x05,
DIETEMP = 0x06,
CURRENT = 0x07,
MANUFACTURER_ID = 0x3e,
DEVICE_ID = 0x3f,
};
#define DEFAULT_MAX_CURRENT 327.68f /* Amps */
#define DEFAULT_SHUNT 0.0003f /* Shunt is 300 uOhm */
// CONFIG register bits
enum CONFIG_BIT : uint16_t {
RST = (1u << 15),
RANGE_HIGH = (0u << 4), // ±163.84 mV — used when R_SHUNT * I_MAX > 40.96 mV
RANGE_LOW = (1u << 4), // ±40.96 mV
};
// DEVICE_ID register field accessor
static constexpr uint16_t DEVICE_ID_MASK = 0xfff0u;
static inline constexpr uint16_t deviceId(uint16_t v) { return (v & DEVICE_ID_MASK) >> 4; }
// ADCCONFIG register bits
enum ADCCONFIG_BIT : uint16_t {
AVERAGES_1 = (0u << 0),
AVERAGES_4 = (1u << 0),
AVERAGES_16 = (2u << 0),
AVERAGES_64 = (3u << 0),
AVERAGES_128 = (4u << 0),
AVERAGES_256 = (5u << 0),
AVERAGES_512 = (6u << 0),
AVERAGES_1024 = (7u << 0),
VTCT_540US = (4u << 3),
VSHCT_540US = (4u << 6),
VBUSCT_540US = (4u << 9),
MODE_TEMP_SHUNT_BUS_CONT = (15u << 12),
};
} // namespace ina238
#define swap16(w) __builtin_bswap16((w))
#define swap32(d) __builtin_bswap32((d))
#define swap64(q) __builtin_bswap64((q))
class INA238 : public device::I2C, public ModuleParams, public I2CSPIDriver<INA238>
{
public:
INA238(const I2CSPIDriverConfig &config, int battery_index);
virtual ~INA238();
~INA238() override;
static I2CSPIDriverBase *instantiate(const I2CSPIDriverConfig &config, int runtime_instance);
static void print_usage();
int init() override;
void RunImpl();
int init() override;
/**
* Tries to call the init() function. If it fails, then it will schedule to retry again in
* INA238_INIT_RETRY_INTERVAL_US microseconds. It will keep retrying at this interval until initialization succeeds.
*
* @return PX4_OK if initialization succeeded on the first try. Negative value otherwise.
*/
int force_init();
/**
* Diagnostics - print some basic information about the driver.
*/
void print_status() override;
protected:
int probe() override;
private:
// Sensor Configuration
struct register_config_t {
Register reg;
uint16_t set_bits{0};
uint16_t clear_bits{0};
enum class State : uint8_t {
UNINITIALIZED, // I2C::init() not yet called successfully — retry until it does
RESET, // soft-reset the device, then transition to CONFIGURE
CONFIGURE, // write SHUNT_CAL / CONFIG / ADCCONFIG, then transition to MEASURE
MEASURE, // steady-state: read VS_BUS / CURRENT / DIETEMP, publish, repeat
};
bool RegisterCheck(const register_config_t &reg_cfg);
int RegisterWrite(Register reg, uint16_t value);
int RegisterRead(Register reg, uint16_t &value);
int Reset();
unsigned int _measure_interval{0};
bool _collect_phase{false};
bool _initialized{false};
perf_counter_t _sample_perf;
perf_counter_t _comms_errors;
perf_counter_t _collection_errors;
perf_counter_t _bad_register_perf{perf_alloc(PC_COUNT, MODULE_NAME": bad register")};
// Configuration state, computed from params
float _max_current;
float _rshunt;
float _current_lsb;
int16_t _range;
uint16_t _shunt_calibration{0};
hrt_abstime _last_config_check_timestamp{0};
uint8_t _checked_register{0};
static constexpr uint8_t size_register_cfg{3};
register_config_t _register_cfg[size_register_cfg] {
// Register | Set bits, Clear bits
{ Register::CONFIG, 0, 0}, // will be set dynamically
{ Register::ADCCONFIG, MODE_TEMP_SHUNT_BUS_CONT | VBUSCT_540US | VSHCT_540US | VTCT_540US | AVERAGES_64},
{ Register::SHUNT_CAL, 0, 0} // will be set dynamically
};
Battery _battery;
uORB::SubscriptionInterval _parameter_update_sub{ORB_ID(parameter_update), 1_s};
uint8_t _connected{0};
// returns state unchanged
bool setConnected(bool state);
int read(uint8_t address, uint16_t &data);
int write(uint8_t address, uint16_t data);
int read(uint8_t address, int16_t &data)
{
return read(address, (uint16_t &)data);
}
int write(uint8_t address, int16_t data)
{
return write(address, (uint16_t)data);
}
/**
* Initialise the automatic measurement state machine and start it.
*
* @note This function is called at open and error time. It might make sense
* to make it more aggressive about resetting the bus in case of errors.
*/
void start();
int collect();
// Rotates through the configuration registers one per call. Returns false
// if a read fails or the value doesn't match what we wrote.
bool checkConfigurationRotating();
int registerRead(ina238::Register reg, uint16_t &value);
int registerWrite(ina238::Register reg, uint16_t value);
// --- State -------------------------------------------------------------
Battery _battery;
State _state{State::UNINITIALIZED};
uint8_t _consecutive_failures{0};
uint8_t _next_reg_to_check{0};
// Configuration computed from params
float _current_lsb{0.f};
uint16_t _shunt_calibration{0};
uint16_t _config_value{0}; // CONFIG register value we wrote
uint16_t _adc_config_value{0}; // ADCCONFIG register value we wrote
// Perf counters
perf_counter_t _sample_perf;
perf_counter_t _comms_errors;
perf_counter_t _collection_errors;
perf_counter_t _bad_register_perf;
perf_counter_t _reinit_perf;
DEFINE_PARAMETERS(
(ParamFloat<px4::params::INA238_CURRENT>) _param_ina238_current,
(ParamFloat<px4::params::INA238_SHUNT>) _param_ina238_shunt
);
};
src/drivers/power_monitor/ina238/ina238_main.cpp
-130
View File
@@ -1,130 +0,0 @@
/****************************************************************************
*
* Copyright (C) 2021 PX4 Development Team. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in
* the documentation and/or other materials provided with the
* distribution.
* 3. Neither the name PX4 nor the names of its contributors may be
* used to endorse or promote products derived from this software
* without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
* OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
* AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
****************************************************************************/
#include <px4_platform_common/getopt.h>
#include <px4_platform_common/module.h>
#include "ina238.h"
I2CSPIDriverBase *INA238::instantiate(const I2CSPIDriverConfig &config, int runtime_instance)
{
INA238 *instance = new INA238(config, config.custom1);
if (instance == nullptr) {
PX4_ERR("alloc failed");
return nullptr;
}
if (config.keep_running) {
if (instance->force_init() != PX4_OK) {
PX4_INFO("Failed to init INA238 on bus %d, but will try again periodically.", config.bus);
}
} else if (instance->init() != PX4_OK) {
delete instance;
return nullptr;
}
return instance;
}
void
INA238::print_usage()
{
PRINT_MODULE_DESCRIPTION(
R"DESCR_STR(
### Description
Driver for the INA238 power monitor.
Multiple instances of this driver can run simultaneously, if each instance has a separate bus OR I2C address.
For example, one instance can run on Bus 2, address 0x45, and one can run on Bus 2, address 0x45.
If the INA238 module is not powered, then by default, initialization of the driver will fail. To change this, use
the -f flag. If this flag is set, then if initialization fails, the driver will keep trying to initialize again
every 0.5 seconds. With this flag set, you can plug in a battery after the driver starts, and it will work. Without
this flag set, the battery must be plugged in before starting the driver.
)DESCR_STR");
PRINT_MODULE_USAGE_NAME("ina238", "driver");
PRINT_MODULE_USAGE_COMMAND("start");
PRINT_MODULE_USAGE_PARAMS_I2C_SPI_DRIVER(true, false);
PRINT_MODULE_USAGE_PARAMS_I2C_ADDRESS(0x45);
PRINT_MODULE_USAGE_PARAMS_I2C_KEEP_RUNNING_FLAG();
PRINT_MODULE_USAGE_PARAM_INT('t', 1, 1, 3, "battery index for calibration values (1 or 3)", true);
PRINT_MODULE_USAGE_DEFAULT_COMMANDS();
}
extern "C" int
ina238_main(int argc, char *argv[])
{
int ch;
using ThisDriver = INA238;
BusCLIArguments cli{true, false};
cli.i2c_address = INA238_BASEADDR;
cli.default_i2c_frequency = 100000;
cli.support_keep_running = true;
cli.custom1 = 1;
while ((ch = cli.getOpt(argc, argv, "t:")) != EOF) {
switch (ch) {
case 't': // battery index
cli.custom1 = (int)strtol(cli.optArg(), NULL, 0);
break;
}
}
const char *verb = cli.optArg();
if (!verb) {
ThisDriver::print_usage();
return -1;
}
BusInstanceIterator iterator(MODULE_NAME, cli, DRV_POWER_DEVTYPE_INA238);
if (!strcmp(verb, "start")) {
return ThisDriver::module_start(cli, iterator);
}
if (!strcmp(verb, "stop")) {
return ThisDriver::module_stop(iterator);
}
if (!strcmp(verb, "status")) {
return ThisDriver::module_status(iterator);
}
ThisDriver::print_usage();
return -1;
}
src/drivers/power_monitor/ina238/ina238_registers.hpp
-91
View File
@@ -1,91 +0,0 @@
//
// Created by roman on 10/18/23.
//
#ifndef PX4_SRC_DRIVERS_POWER_MONITOR_INA238_INA238_REGISTERS_HPP_
#define PX4_SRC_DRIVERS_POWER_MONITOR_INA238_INA238_REGISTERS_HPP_
namespace ina238
{
enum class Register : uint8_t {
CONFIG = 0x00, // Configuration Register
ADCCONFIG = 0x01, // ADC Configuration Register
SHUNT_CAL = 0x02, // Shunt Calibration Register
VS_BUS = 0x05,
DIETEMP = 0x06,
CURRENT = 0x07,
MANUFACTURER_ID = 0x3e,
DEVICE_ID = 0x3f
};
enum CONFIG_BIT : uint16_t {
RANGE_BIT = (1 << 4),
ADC_RESET_BIT = (1 << 15)
};
enum ADCCONFIG_BIT : uint16_t {
AVERAGES_1 = (0 << 0),
AVERAGES_4 = (1 << 0),
AVERAGES_16 = (2 << 0),
AVERAGES_64 = (3 << 0),
AVERAGES_128 = (4 << 0),
AVERAGES_256 = (5 << 0),
AVERAGES_512 = (6 << 0),
AVERAGES_1024 = (7 << 0),
VTCT_50US = (0 << 3),
VTCT_84US = (1 << 3),
VTCT_150US = (2 << 3),
VTCT_280US = (3 << 3),
VTCT_540US = (4 << 3),
VTCT_1052US = (5 << 3),
VTCT_2074US = (6 << 3),
VTCT_4170US = (7 << 3),
VSHCT_MASK = (7 << 6),
VSHCT_50US = (0 << 6),
VSHCT_84US = (1 << 6),
VSHCT_150US = (2 << 6),
VSHCT_280US = (3 << 6),
VSHCT_540US = (4 << 6),
VSHCT_1052US = (5 << 6),
VSHCT_2074US = (6 << 6),
VSHCT_4170US = (7 << 6),
VBUSCT_MASK = (7 << 9),
VBUSCT_50US = (0 << 9),
VBUSCT_84US = (1 << 9),
VBUSCT_150US = (2 << 9),
VBUSCT_280US = (3 << 9),
VBUSCT_540US = (4 << 9),
VBUSCT_1052US = (5 << 9),
VBUSCT_2074US = (6 << 9),
VBUSCT_4170US = (7 << 9),
MODE_SHUTDOWN_TRIG = (0 << 12),
MODE_BUS_TRIG = (1 << 12),
MODE_SHUNT_TRIG = (2 << 12),
MODE_SHUNT_BUS_TRIG = (3 << 12),
MODE_TEMP_TRIG = (4 << 12),
MODE_TEMP_BUS_TRIG = (5 << 12),
MODE_TEMP_SHUNT_TRIG = (6 << 12),
MODE_TEMP_SHUNT_BUS_TRIG = (7 << 12),
MODE_SHUTDOWN_CONT = (8 << 12),
MODE_BUS_CONT = (9 << 12),
MODE_SHUNT_CONT = (10 << 12),
MODE_SHUNT_BUS_CONT = (11 << 12),
MODE_TEMP_CONT = (12 << 12),
MODE_TEMP_BUS_CONT = (13 << 12),
MODE_TEMP_SHUNT_CONT = (14 << 12),
MODE_TEMP_SHUNT_BUS_CONT = (15 << 12)
};
}
#endif //PX4_SRC_DRIVERS_POWER_MONITOR_INA238_INA238_REGISTERS_HPP_