From 3d8cfe12645aef1eee96e7fd709ba013cf09f9b8 Mon Sep 17 00:00:00 2001 From: alexklimaj Date: Wed, 1 Jan 2025 19:50:51 -0700 Subject: [PATCH 1/6] ina238: retry if read fails --- src/drivers/power_monitor/ina238/ina238.cpp | 34 ++++++++++++--------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/drivers/power_monitor/ina238/ina238.cpp b/src/drivers/power_monitor/ina238/ina238.cpp index 42f5e1285849..e4c552a3a605 100644 --- a/src/drivers/power_monitor/ina238/ina238.cpp +++ b/src/drivers/power_monitor/ina238/ina238.cpp @@ -96,14 +96,19 @@ int INA238::read(uint8_t address, uint16_t &data) { // read desired little-endian value via I2C uint16_t received_bytes; - const int ret = transfer(&address, 1, (uint8_t *)&received_bytes, sizeof(received_bytes)); + int ret = PX4_ERROR; - if (ret == PX4_OK) { - data = swap16(received_bytes); + for (size_t i = 0; i < 3; i++) { + ret = transfer(&address, 1, (uint8_t *)&received_bytes, sizeof(received_bytes)); - } else { - perf_count(_comms_errors); - PX4_DEBUG("i2c::transfer returned %d", ret); + if (ret == PX4_OK) { + data = swap16(received_bytes); + break; + + } else { + perf_count(_comms_errors); + PX4_DEBUG("i2c::transfer returned %d", ret); + } } return ret; @@ -231,6 +236,12 @@ int INA238::collect() success = success && (RegisterRead(Register::CURRENT, (uint16_t &)current) == PX4_OK); success = success && (RegisterRead(Register::DIETEMP, (uint16_t &)temperature) == PX4_OK); + if (success) { + _battery.updateVoltage(static_cast(bus_voltage * INA238_VSCALE)); + _battery.updateCurrent(static_cast(current * _current_lsb)); + _battery.updateTemperature(static_cast(temperature * INA238_TSCALE)); + } + if (!success || hrt_elapsed_time(&_last_config_check_timestamp) > 100_ms) { // check configuration registers periodically or immediately following any failure if (RegisterCheck(_register_cfg[_checked_register])) { @@ -245,15 +256,8 @@ int INA238::collect() } } - if (!success) { - PX4_DEBUG("error reading from sensor"); - bus_voltage = current = 0; - } - _battery.setConnected(success); - _battery.updateVoltage(static_cast(bus_voltage * INA238_VSCALE)); - _battery.updateCurrent(static_cast(current * _current_lsb)); - _battery.updateTemperature(static_cast(temperature * INA238_TSCALE)); + _battery.updateAndPublishBatteryStatus(hrt_absolute_time()); perf_end(_sample_perf); @@ -262,6 +266,8 @@ int INA238::collect() return PX4_OK; } else { + PX4_DEBUG("error reading from sensor"); + return PX4_ERROR; } } From 15b94a38983e02f2e9c59562e5482e4431aaaf83 Mon Sep 17 00:00:00 2001 From: alexklimaj Date: Mon, 6 Jan 2025 15:50:31 -0700 Subject: [PATCH 2/6] ina238: increase retries and only publish not connected if register check fails --- src/drivers/power_monitor/ina238/ina238.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/drivers/power_monitor/ina238/ina238.cpp b/src/drivers/power_monitor/ina238/ina238.cpp index e4c552a3a605..df7d0079fc04 100644 --- a/src/drivers/power_monitor/ina238/ina238.cpp +++ b/src/drivers/power_monitor/ina238/ina238.cpp @@ -98,7 +98,7 @@ int INA238::read(uint8_t address, uint16_t &data) uint16_t received_bytes; int ret = PX4_ERROR; - for (size_t i = 0; i < 3; i++) { + for (size_t i = 0; i < 6; i++) { ret = transfer(&address, 1, (uint8_t *)&received_bytes, sizeof(received_bytes)); if (ret == PX4_OK) { @@ -240,6 +240,10 @@ int INA238::collect() _battery.updateVoltage(static_cast(bus_voltage * INA238_VSCALE)); _battery.updateCurrent(static_cast(current * _current_lsb)); _battery.updateTemperature(static_cast(temperature * INA238_TSCALE)); + + _battery.setConnected(success); + + _battery.updateAndPublishBatteryStatus(hrt_absolute_time()); } if (!success || hrt_elapsed_time(&_last_config_check_timestamp) > 100_ms) { @@ -253,12 +257,12 @@ int INA238::collect() PX4_DEBUG("register check failed"); perf_count(_bad_register_perf); success = false; - } - } - _battery.setConnected(success); + _battery.setConnected(success); - _battery.updateAndPublishBatteryStatus(hrt_absolute_time()); + _battery.updateAndPublishBatteryStatus(hrt_absolute_time()); + } + } perf_end(_sample_perf); From 4a81e659295555e5c0b4183f0b5d6286d121da83 Mon Sep 17 00:00:00 2001 From: alexklimaj Date: Wed, 22 Jan 2025 09:59:04 -0700 Subject: [PATCH 3/6] ina238: use I2C resets --- src/drivers/power_monitor/ina238/ina238.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/drivers/power_monitor/ina238/ina238.cpp b/src/drivers/power_monitor/ina238/ina238.cpp index df7d0079fc04..233d34b695cd 100644 --- a/src/drivers/power_monitor/ina238/ina238.cpp +++ b/src/drivers/power_monitor/ina238/ina238.cpp @@ -98,17 +98,13 @@ int INA238::read(uint8_t address, uint16_t &data) uint16_t received_bytes; int ret = PX4_ERROR; - for (size_t i = 0; i < 6; i++) { - ret = transfer(&address, 1, (uint8_t *)&received_bytes, sizeof(received_bytes)); + ret = transfer(&address, 1, (uint8_t *)&received_bytes, sizeof(received_bytes)); - if (ret == PX4_OK) { - data = swap16(received_bytes); - break; - - } else { - perf_count(_comms_errors); - PX4_DEBUG("i2c::transfer returned %d", ret); - } + if (ret == PX4_OK) { + data = swap16(received_bytes); + } else { + perf_count(_comms_errors); + PX4_DEBUG("i2c::transfer returned %d", ret); } return ret; @@ -165,6 +161,8 @@ int INA238::Reset() int ret = PX4_ERROR; + _retries = 6; + if (RegisterWrite(Register::CONFIG, (uint16_t)(ADC_RESET_BIT)) != PX4_OK) { return ret; } From 5097214227af74b65da1b32342e5d7ed5657482f Mon Sep 17 00:00:00 2001 From: Daniel Agar Date: Wed, 22 Jan 2025 13:21:27 -0500 Subject: [PATCH 4/6] Apply suggestions from code review --- src/drivers/power_monitor/ina238/ina238.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/drivers/power_monitor/ina238/ina238.cpp b/src/drivers/power_monitor/ina238/ina238.cpp index 233d34b695cd..07e6dadd023f 100644 --- a/src/drivers/power_monitor/ina238/ina238.cpp +++ b/src/drivers/power_monitor/ina238/ina238.cpp @@ -96,9 +96,7 @@ int INA238::read(uint8_t address, uint16_t &data) { // read desired little-endian value via I2C uint16_t received_bytes; - int ret = PX4_ERROR; - - ret = transfer(&address, 1, (uint8_t *)&received_bytes, sizeof(received_bytes)); + const int ret = transfer(&address, 1, (uint8_t *)&received_bytes, sizeof(received_bytes)); if (ret == PX4_OK) { data = swap16(received_bytes); From e5cebc459a2bb05e03b02ef446869d75f04b5ded Mon Sep 17 00:00:00 2001 From: Daniel Agar Date: Wed, 22 Jan 2025 13:23:04 -0500 Subject: [PATCH 5/6] fix astyle --- src/drivers/power_monitor/ina238/ina238.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/drivers/power_monitor/ina238/ina238.cpp b/src/drivers/power_monitor/ina238/ina238.cpp index 07e6dadd023f..6edf56d2223a 100644 --- a/src/drivers/power_monitor/ina238/ina238.cpp +++ b/src/drivers/power_monitor/ina238/ina238.cpp @@ -100,6 +100,7 @@ int INA238::read(uint8_t address, uint16_t &data) if (ret == PX4_OK) { data = swap16(received_bytes); + } else { perf_count(_comms_errors); PX4_DEBUG("i2c::transfer returned %d", ret); From 4764703e0da2518620ad40b717495f2b7ae5a4bd Mon Sep 17 00:00:00 2001 From: Daniel Agar Date: Wed, 22 Jan 2025 13:35:25 -0500 Subject: [PATCH 6/6] Update src/drivers/power_monitor/ina238/ina238.cpp --- src/drivers/power_monitor/ina238/ina238.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/power_monitor/ina238/ina238.cpp b/src/drivers/power_monitor/ina238/ina238.cpp index 6edf56d2223a..b46b277a8435 100644 --- a/src/drivers/power_monitor/ina238/ina238.cpp +++ b/src/drivers/power_monitor/ina238/ina238.cpp @@ -160,7 +160,7 @@ int INA238::Reset() int ret = PX4_ERROR; - _retries = 6; + _retries = 3; if (RegisterWrite(Register::CONFIG, (uint16_t)(ADC_RESET_BIT)) != PX4_OK) { return ret;