From 09edf66a2a00c20bce65203c84c6081c9d129aa7 Mon Sep 17 00:00:00 2001 From: Shane Synan Date: Sun, 26 Feb 2023 12:23:39 -0500 Subject: [PATCH] FuriHal, Power, UnitTests: fix, rename battery charging voltage limit API (#2228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * FuriHal, Power, UnitTests: rename battery charge voltage limit API * FuriHal: bump API, power info major versions * Power: fix battery charge voltage limit for > 7.935v Co-authored-by: あく --- .../debug/unit_tests/power/power_test.c | 61 +++++++++++-------- .../services/power/power_service/power.c | 6 +- .../services/power/power_service/power.h | 2 +- .../power_settings_scene_battery_info.c | 2 +- .../power_settings_app/views/battery_info.c | 6 +- .../power_settings_app/views/battery_info.h | 2 +- firmware/targets/f7/api_symbols.csv | 4 +- firmware/targets/f7/furi_hal/furi_hal_power.c | 12 ++-- .../targets/furi_hal_include/furi_hal_power.h | 10 +-- lib/drivers/bq25896.c | 17 +++--- lib/drivers/bq25896.h | 4 +- 11 files changed, 69 insertions(+), 57 deletions(-) diff --git a/applications/debug/unit_tests/power/power_test.c b/applications/debug/unit_tests/power/power_test.c index ce2c7aad..a9b66b22 100644 --- a/applications/debug/unit_tests/power/power_test.c +++ b/applications/debug/unit_tests/power/power_test.c @@ -3,56 +3,63 @@ #include "../minunit.h" static void power_test_deinit(void) { - // Try to reset to default charging voltage - furi_hal_power_set_battery_charging_voltage(4.208f); + // Try to reset to default charge voltage limit + furi_hal_power_set_battery_charge_voltage_limit(4.208f); } -MU_TEST(test_power_charge_voltage_exact) { - // Power of 16mV charge voltages get applied exactly +MU_TEST(test_power_charge_voltage_limit_exact) { + // Power of 16mV charge voltage limits get applied exactly // (bq25896 charge controller works in 16mV increments) // // This test may need adapted if other charge controllers are used in the future. for(uint16_t charge_mv = 3840; charge_mv <= 4208; charge_mv += 16) { float charge_volt = (float)charge_mv / 1000.0f; - furi_hal_power_set_battery_charging_voltage(charge_volt); - mu_assert_double_eq(charge_volt, furi_hal_power_get_battery_charging_voltage()); + furi_hal_power_set_battery_charge_voltage_limit(charge_volt); + mu_assert_double_eq(charge_volt, furi_hal_power_get_battery_charge_voltage_limit()); } } -MU_TEST(test_power_charge_voltage_floating_imprecision) { +MU_TEST(test_power_charge_voltage_limit_floating_imprecision) { // 4.016f should act as 4.016 V, even with floating point imprecision - furi_hal_power_set_battery_charging_voltage(4.016f); - mu_assert_double_eq(4.016f, furi_hal_power_get_battery_charging_voltage()); + furi_hal_power_set_battery_charge_voltage_limit(4.016f); + mu_assert_double_eq(4.016f, furi_hal_power_get_battery_charge_voltage_limit()); } -MU_TEST(test_power_charge_voltage_inexact) { - // Charge voltages that are not power of 16mV get truncated down - furi_hal_power_set_battery_charging_voltage(3.841f); - mu_assert_double_eq(3.840, furi_hal_power_get_battery_charging_voltage()); +MU_TEST(test_power_charge_voltage_limit_inexact) { + // Charge voltage limits that are not power of 16mV get truncated down + furi_hal_power_set_battery_charge_voltage_limit(3.841f); + mu_assert_double_eq(3.840, furi_hal_power_get_battery_charge_voltage_limit()); - furi_hal_power_set_battery_charging_voltage(3.900f); - mu_assert_double_eq(3.888, furi_hal_power_get_battery_charging_voltage()); + furi_hal_power_set_battery_charge_voltage_limit(3.900f); + mu_assert_double_eq(3.888, furi_hal_power_get_battery_charge_voltage_limit()); - furi_hal_power_set_battery_charging_voltage(4.200f); - mu_assert_double_eq(4.192, furi_hal_power_get_battery_charging_voltage()); + furi_hal_power_set_battery_charge_voltage_limit(4.200f); + mu_assert_double_eq(4.192, furi_hal_power_get_battery_charge_voltage_limit()); } -MU_TEST(test_power_charge_voltage_invalid_clamped) { - // Out-of-range charge voltages get clamped to 3.840 V and 4.208 V - furi_hal_power_set_battery_charging_voltage(3.808f); - mu_assert_double_eq(3.840, furi_hal_power_get_battery_charging_voltage()); +MU_TEST(test_power_charge_voltage_limit_invalid_clamped) { + // Out-of-range charge voltage limits get clamped to 3.840 V and 4.208 V + furi_hal_power_set_battery_charge_voltage_limit(3.808f); + mu_assert_double_eq(3.840, furi_hal_power_get_battery_charge_voltage_limit()); + furi_hal_power_set_battery_charge_voltage_limit(1.0f); + mu_assert_double_eq(3.840, furi_hal_power_get_battery_charge_voltage_limit()); // NOTE: Intentionally picking a small increment above 4.208 V to reduce the risk of an // unhappy battery if this fails. - furi_hal_power_set_battery_charging_voltage(4.240f); - mu_assert_double_eq(4.208, furi_hal_power_get_battery_charging_voltage()); + furi_hal_power_set_battery_charge_voltage_limit(4.240f); + mu_assert_double_eq(4.208, furi_hal_power_get_battery_charge_voltage_limit()); + // Likewise, picking a number that the uint8_t wraparound in the driver would result in a + // VREG value under 23 if this test fails. + // E.g. (uint8_t)((8105-3840)/16) -> 10 + furi_hal_power_set_battery_charge_voltage_limit(8.105f); + mu_assert_double_eq(4.208, furi_hal_power_get_battery_charge_voltage_limit()); } MU_TEST_SUITE(test_power_suite) { - MU_RUN_TEST(test_power_charge_voltage_exact); - MU_RUN_TEST(test_power_charge_voltage_floating_imprecision); - MU_RUN_TEST(test_power_charge_voltage_inexact); - MU_RUN_TEST(test_power_charge_voltage_invalid_clamped); + MU_RUN_TEST(test_power_charge_voltage_limit_exact); + MU_RUN_TEST(test_power_charge_voltage_limit_floating_imprecision); + MU_RUN_TEST(test_power_charge_voltage_limit_inexact); + MU_RUN_TEST(test_power_charge_voltage_limit_invalid_clamped); power_test_deinit(); } diff --git a/applications/services/power/power_service/power.c b/applications/services/power/power_service/power.c index d9319d3d..56dbd0f8 100644 --- a/applications/services/power/power_service/power.c +++ b/applications/services/power/power_service/power.c @@ -13,8 +13,8 @@ void power_draw_battery_callback(Canvas* canvas, void* context) { if(power->info.gauge_is_ok) { canvas_draw_box(canvas, 2, 2, (power->info.charge + 4) / 5, 4); - if(power->info.voltage_battery_charging < 4.2) { - // Battery charging voltage is modified, indicate with cross pattern + if(power->info.voltage_battery_charge_limit < 4.2) { + // Battery charge voltage limit is modified, indicate with cross pattern canvas_invert_color(canvas); uint8_t battery_bar_width = (power->info.charge + 4) / 5; bool cross_odd = false; @@ -147,7 +147,7 @@ static bool power_update_info(Power* power) { info.capacity_full = furi_hal_power_get_battery_full_capacity(); info.current_charger = furi_hal_power_get_battery_current(FuriHalPowerICCharger); info.current_gauge = furi_hal_power_get_battery_current(FuriHalPowerICFuelGauge); - info.voltage_battery_charging = furi_hal_power_get_battery_charging_voltage(); + info.voltage_battery_charge_limit = furi_hal_power_get_battery_charge_voltage_limit(); info.voltage_charger = furi_hal_power_get_battery_voltage(FuriHalPowerICCharger); info.voltage_gauge = furi_hal_power_get_battery_voltage(FuriHalPowerICFuelGauge); info.voltage_vbus = furi_hal_power_get_usb_voltage(); diff --git a/applications/services/power/power_service/power.h b/applications/services/power/power_service/power.h index 8b9019c4..c7f5d7e3 100644 --- a/applications/services/power/power_service/power.h +++ b/applications/services/power/power_service/power.h @@ -41,7 +41,7 @@ typedef struct { float current_charger; float current_gauge; - float voltage_battery_charging; + float voltage_battery_charge_limit; float voltage_charger; float voltage_gauge; float voltage_vbus; diff --git a/applications/settings/power_settings_app/scenes/power_settings_scene_battery_info.c b/applications/settings/power_settings_app/scenes/power_settings_scene_battery_info.c index 5fa38df7..5181c93f 100644 --- a/applications/settings/power_settings_app/scenes/power_settings_scene_battery_info.c +++ b/applications/settings/power_settings_app/scenes/power_settings_scene_battery_info.c @@ -7,7 +7,7 @@ static void power_settings_scene_battery_info_update_model(PowerSettingsApp* app .gauge_voltage = app->info.voltage_gauge, .gauge_current = app->info.current_gauge, .gauge_temperature = app->info.temperature_gauge, - .charging_voltage = app->info.voltage_battery_charging, + .charge_voltage_limit = app->info.voltage_battery_charge_limit, .charge = app->info.charge, .health = app->info.health, }; diff --git a/applications/settings/power_settings_app/views/battery_info.c b/applications/settings/power_settings_app/views/battery_info.c index d29769d2..7394fd3c 100644 --- a/applications/settings/power_settings_app/views/battery_info.c +++ b/applications/settings/power_settings_app/views/battery_info.c @@ -69,7 +69,7 @@ static void draw_battery(Canvas* canvas, BatteryInfoModel* data, int x, int y) { drain_current > HIGH_DRAIN_CURRENT_THRESHOLD ? "mA!" : "mA"); } else if(drain_current != 0) { snprintf(header, 20, "..."); - } else if(data->charging_voltage < 4.2) { + } else if(data->charge_voltage_limit < 4.2) { // Non-default battery charging limit, mention it snprintf(emote, sizeof(emote), "Charged!"); snprintf(header, sizeof(header), "Limited to"); @@ -77,8 +77,8 @@ static void draw_battery(Canvas* canvas, BatteryInfoModel* data, int x, int y) { value, sizeof(value), "%lu.%luV", - (uint32_t)(data->charging_voltage), - (uint32_t)(data->charging_voltage * 10) % 10); + (uint32_t)(data->charge_voltage_limit), + (uint32_t)(data->charge_voltage_limit * 10) % 10); } else { snprintf(header, sizeof(header), "Charged!"); } diff --git a/applications/settings/power_settings_app/views/battery_info.h b/applications/settings/power_settings_app/views/battery_info.h index 7bfacf69..e52d1844 100644 --- a/applications/settings/power_settings_app/views/battery_info.h +++ b/applications/settings/power_settings_app/views/battery_info.h @@ -9,7 +9,7 @@ typedef struct { float gauge_voltage; float gauge_current; float gauge_temperature; - float charging_voltage; + float charge_voltage_limit; uint8_t charge; uint8_t health; } BatteryInfoModel; diff --git a/firmware/targets/f7/api_symbols.csv b/firmware/targets/f7/api_symbols.csv index e320fc92..8a76f8c9 100644 --- a/firmware/targets/f7/api_symbols.csv +++ b/firmware/targets/f7/api_symbols.csv @@ -1211,7 +1211,7 @@ Function,+,furi_hal_power_enable_external_3_3v,void, Function,+,furi_hal_power_enable_otg,void, Function,+,furi_hal_power_gauge_is_ok,_Bool, Function,+,furi_hal_power_get_bat_health_pct,uint8_t, -Function,+,furi_hal_power_get_battery_charging_voltage,float, +Function,+,furi_hal_power_get_battery_charge_voltage_limit,float, Function,+,furi_hal_power_get_battery_current,float,FuriHalPowerIC Function,+,furi_hal_power_get_battery_design_capacity,uint32_t, Function,+,furi_hal_power_get_battery_full_capacity,uint32_t, @@ -1230,7 +1230,7 @@ Function,+,furi_hal_power_is_charging_done,_Bool, Function,+,furi_hal_power_is_otg_enabled,_Bool, Function,+,furi_hal_power_off,void, Function,+,furi_hal_power_reset,void, -Function,+,furi_hal_power_set_battery_charging_voltage,void,float +Function,+,furi_hal_power_set_battery_charge_voltage_limit,void,float Function,+,furi_hal_power_shutdown,void, Function,+,furi_hal_power_sleep,void, Function,+,furi_hal_power_sleep_available,_Bool, diff --git a/firmware/targets/f7/furi_hal/furi_hal_power.c b/firmware/targets/f7/furi_hal/furi_hal_power.c index 2d709620..dd7c34ae 100644 --- a/firmware/targets/f7/furi_hal/furi_hal_power.c +++ b/firmware/targets/f7/furi_hal/furi_hal_power.c @@ -341,14 +341,14 @@ bool furi_hal_power_is_otg_enabled() { return ret; } -float furi_hal_power_get_battery_charging_voltage() { +float furi_hal_power_get_battery_charge_voltage_limit() { furi_hal_i2c_acquire(&furi_hal_i2c_handle_power); float ret = (float)bq25896_get_vreg_voltage(&furi_hal_i2c_handle_power) / 1000.0f; furi_hal_i2c_release(&furi_hal_i2c_handle_power); return ret; } -void furi_hal_power_set_battery_charging_voltage(float voltage) { +void furi_hal_power_set_battery_charge_voltage_limit(float voltage) { furi_hal_i2c_acquire(&furi_hal_i2c_handle_power); // Adding 0.0005 is necessary because 4.016f is 4.015999794000, which gets truncated bq25896_set_vreg_voltage(&furi_hal_i2c_handle_power, (uint16_t)(voltage * 1000.0f + 0.0005f)); @@ -486,7 +486,7 @@ void furi_hal_power_info_get(PropertyValueCallback out, char sep, void* context) property_value_out(&property_context, NULL, 2, "format", "major", "2"); property_value_out(&property_context, NULL, 2, "format", "minor", "1"); } else { - property_value_out(&property_context, NULL, 3, "power", "info", "major", "1"); + property_value_out(&property_context, NULL, 3, "power", "info", "major", "2"); property_value_out(&property_context, NULL, 3, "power", "info", "minor", "1"); } @@ -505,8 +505,10 @@ void furi_hal_power_info_get(PropertyValueCallback out, char sep, void* context) } property_value_out(&property_context, NULL, 2, "charge", "state", charge_state); - uint16_t charge_voltage = (uint16_t)(furi_hal_power_get_battery_charging_voltage() * 1000.f); - property_value_out(&property_context, "%u", 2, "charge", "voltage", charge_voltage); + uint16_t charge_voltage_limit = + (uint16_t)(furi_hal_power_get_battery_charge_voltage_limit() * 1000.f); + property_value_out( + &property_context, "%u", 3, "charge", "voltage", "limit", charge_voltage_limit); uint16_t voltage = (uint16_t)(furi_hal_power_get_battery_voltage(FuriHalPowerICFuelGauge) * 1000.f); property_value_out(&property_context, "%u", 2, "battery", "voltage", voltage); diff --git a/firmware/targets/furi_hal_include/furi_hal_power.h b/firmware/targets/furi_hal_include/furi_hal_power.h index 39a11e99..462e20e4 100644 --- a/firmware/targets/furi_hal_include/furi_hal_power.h +++ b/firmware/targets/furi_hal_include/furi_hal_power.h @@ -121,21 +121,21 @@ void furi_hal_power_check_otg_status(); */ bool furi_hal_power_is_otg_enabled(); -/** Get battery charging voltage in V +/** Get battery charge voltage limit in V * * @return voltage in V */ -float furi_hal_power_get_battery_charging_voltage(); +float furi_hal_power_get_battery_charge_voltage_limit(); -/** Set battery charging voltage in V +/** Set battery charge voltage limit in V * - * Invalid values will be clamped to the nearest valid value. + * Invalid values will be clamped downward to the nearest valid value. * * @param voltage[in] voltage in V * * @return voltage in V */ -void furi_hal_power_set_battery_charging_voltage(float voltage); +void furi_hal_power_set_battery_charge_voltage_limit(float voltage); /** Get remaining battery battery capacity in mAh * diff --git a/lib/drivers/bq25896.c b/lib/drivers/bq25896.c index 7e3008d6..99534fb1 100644 --- a/lib/drivers/bq25896.c +++ b/lib/drivers/bq25896.c @@ -140,15 +140,18 @@ uint16_t bq25896_get_vreg_voltage(FuriHalI2cBusHandle* handle) { void bq25896_set_vreg_voltage(FuriHalI2cBusHandle* handle, uint16_t vreg_voltage) { if(vreg_voltage < 3840) { - // Minimum value is 3840 mV - bq25896_regs.r06.VREG = 0; - } else { - // Find the nearest voltage value (subtract offset, divide into sections) - // Values are truncated downward as needed (e.g. 4200mV -> 4192 mV) - bq25896_regs.r06.VREG = (uint8_t)((vreg_voltage - 3840) / 16); + // Minimum valid value is 3840 mV + vreg_voltage = 3840; + } else if(vreg_voltage > 4208) { + // Maximum safe value is 4208 mV + vreg_voltage = 4208; } - // Do not allow values above 23 (0x17, 4208mV) + // Find the nearest voltage value (subtract offset, divide into sections) + // Values are truncated downward as needed (e.g. 4200mV -> 4192 mV) + bq25896_regs.r06.VREG = (uint8_t)((vreg_voltage - 3840) / 16); + + // Double check: do not allow values above 23 (0x17, 4208mV) // Exceeding 4.2v will overcharge the battery! if(bq25896_regs.r06.VREG > 23) { bq25896_regs.r06.VREG = 23; diff --git a/lib/drivers/bq25896.h b/lib/drivers/bq25896.h index c8a8526a..f3d1d0e0 100644 --- a/lib/drivers/bq25896.h +++ b/lib/drivers/bq25896.h @@ -36,10 +36,10 @@ void bq25896_disable_otg(FuriHalI2cBusHandle* handle); /** Is otg enabled */ bool bq25896_is_otg_enabled(FuriHalI2cBusHandle* handle); -/** Get VREG (charging) voltage in mV */ +/** Get VREG (charging limit) voltage in mV */ uint16_t bq25896_get_vreg_voltage(FuriHalI2cBusHandle* handle); -/** Set VREG (charging) voltage in mV +/** Set VREG (charging limit) voltage in mV * * Valid range: 3840mV - 4208mV, in steps of 16mV */