patrick/zephyr
1
0
Fork 0
Code Issues Pull requests Actions 8 Packages Projects Releases Wiki Activity

fuel_gauge: Remove status from fuel gauge properties

Browse source 
Based on review of the similar charger driver API, it's been demonstrated
from the community that embedding a per value status code when fetching
multiple properties isn't particularly wanted or needed. It was largely
considered not worth the additional maintenance to have the extra per
property error information.

Remove the status field from the fuel_gauge property value structs.

Signed-off-by: Aaron Massey <aaronmassey@google.com>
This commit is contained in:
Aaron Massey 2023-09-21 11:38:39 -06:00 committed by Carles Cufí
parent bddd88955d
commit 329ecd1e12
8 changed files with 34 additions and 206 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
drivers/fuel_gauge/bq27z746/bq27z746.c
View file

@ -192,8 +192,6 @@ static int bq27z746_get_prop(const struct device *dev, struct fuel_gauge_propert
rc = -ENOTSUP;
}
prop->status = rc;
return rc;
}
@ -232,7 +230,6 @@ static int bq27z746_get_buffer_prop(const struct device *dev,
rc = -ENOTSUP;
}
prop->status = rc;
return rc;
}
@ -255,8 +252,6 @@ static int bq27z746_set_prop(const struct device *dev, struct fuel_gauge_propert
rc = -ENOTSUP;
}
prop->status = rc;
return rc;
}

2
drivers/fuel_gauge/max17048/max17048.c
View file

@ -197,8 +197,6 @@ static int max17048_get_single_prop_impl(const struct device *dev, struct fuel_g
rc = -ENOTSUP;
}
prop->status = rc;
return rc;
}

5
drivers/fuel_gauge/sbs_gauge/sbs_gauge.c
View file

@ -172,8 +172,6 @@ static int sbs_gauge_get_prop(const struct device *dev, struct fuel_gauge_proper
rc = -ENOTSUP;
}
prop->status = rc;
return rc;
}
@ -230,8 +228,6 @@ static int sbs_gauge_set_prop(const struct device *dev, struct fuel_gauge_proper
rc = -ENOTSUP;
}
prop->status = rc;
return rc;
}
@ -270,7 +266,6 @@ static int sbs_gauge_get_buffer_prop(const struct device *dev,
rc = -ENOTSUP;
}
prop->status = rc;
return rc;
}

33
include/zephyr/drivers/fuel_gauge.h
View file

@ -119,9 +119,6 @@ struct fuel_gauge_property {
/** Battery fuel gauge property to get */
fuel_gauge_prop_t property_type;
/** Negative error status set by callee e.g. -ENOTSUP for an unsupported property */
int status;
/** Property field for getting */
union {
/* Fields have the format: */
@ -188,9 +185,6 @@ struct fuel_gauge_property {
struct fuel_gauge_buffer_property {
/** Battery fuel gauge property to get */
fuel_gauge_prop_t property_type;
/** Negative error status set by callee e.g. -ENOTSUP for an unsupported property */
int status;
};
/**
@ -303,26 +297,24 @@ static inline int z_impl_fuel_gauge_get_prop(const struct device *dev,
* maintains the same order of properties as it was given.
* @param len number of properties in props array
*
* @return return=0 if successful, return < 0 if getting all properties failed, return > 0 if some
* properties failed where return=number of failing properties.
* @return 0 if successful, negative errno code of first failing property
*/
__syscall int fuel_gauge_get_props(const struct device *dev, struct fuel_gauge_property *props,
size_t len);
static inline int z_impl_fuel_gauge_get_props(const struct device *dev,
struct fuel_gauge_property *props, size_t len)
{
int err_count = 0;
const struct fuel_gauge_driver_api *api = dev->api;
for (int i = 0; i < len; i++) {
int ret = api->get_property(dev, props + i);
err_count += ret ? 1 : 0;
if (ret) {
return ret;
}
}
err_count = (err_count == len) ? -1 : err_count;
return err_count;
return 0;
}
/**
@ -333,7 +325,7 @@ static inline int z_impl_fuel_gauge_get_props(const struct device *dev,
* field is set by the caller to determine what property is written to the fuel gauge device from
* the fuel_gauge_property struct's value field.
*
* @return 0 if successful, negative errno code if failure.
* @return 0 if successful, negative errno code of first failing property
*/
__syscall int fuel_gauge_set_prop(const struct device *dev, struct fuel_gauge_property *prop);
@ -358,8 +350,7 @@ static inline int z_impl_fuel_gauge_set_prop(const struct device *dev,
* the fuel_gauge_property struct's value field.
* @param props_len number of properties in props array
*
* @return return=0 if successful, return < 0 if setting all properties failed, return > 0 if some
* properties failed where return=number of failing properties.
* @return return=0 if successful. Otherwise, return array index of failing property.
*/
__syscall int fuel_gauge_set_props(const struct device *dev, struct fuel_gauge_property *props,
size_t props_len);
@ -367,17 +358,15 @@ __syscall int fuel_gauge_set_props(const struct device *dev, struct fuel_gauge_p
static inline int z_impl_fuel_gauge_set_props(const struct device *dev,
struct fuel_gauge_property *props, size_t props_len)
{
int err_count = 0;
for (int i = 0; i < props_len; i++) {
int ret = fuel_gauge_set_prop(dev, props + i);
err_count += ret ? 1 : 0;
if (ret) {
return ret;
}
}
err_count = (err_count == props_len) ? -1 : err_count;
return err_count;
return 0;
}
/**

36
samples/fuel_gauge/max17048/src/main.c
View file

@ -50,54 +50,20 @@ int main(void)
},
{
.property_type = FUEL_GAUGE_VOLTAGE,
}
};
}};
ret = fuel_gauge_get_props(dev, props, ARRAY_SIZE(props));
if (ret < 0) {
printk("Error: cannot get properties\n");
} else {
if (ret != 0) {
printk("Warning: Some properties failed\n");
}
if (props[0].status == 0) {
printk("Time to empty %d\n", props[0].value.runtime_to_empty);
} else {
printk(
"Property FUEL_GAUGE_RUNTIME_TO_EMPTY failed with error %d\n",
props[0].status
);
}
if (props[1].status == 0) {
printk("Time to full %d\n", props[1].value.runtime_to_full);
} else {
printk(
"Property FUEL_GAUGE_RUNTIME_TO_FULL failed with error %d\n",
props[1].status
);
}
if (props[2].status == 0) {
printk("Charge %d%%\n", props[2].value.relative_state_of_charge);
} else {
printk(
"Property FUEL_GAUGE_STATE_OF_CHARGE failed with error %d\n",
props[2].status
);
}
if (props[3].status == 0) {
printk("Voltage %d\n", props[3].value.voltage);
} else {
printk(
"Property FUEL_GAUGE_VOLTAGE failed with error %d\n",
props[3].status
);
}
}
k_sleep(K_MSEC(5000));
}

29
tests/drivers/fuel_gauge/bq27z746/src/test_bq27z746.c
View file

@ -42,10 +42,7 @@ ZTEST_USER_F(bq27z746, test_get_all_props_failed_returns_negative)
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
zassert_equal(props[0].status, -ENOTSUP, "Getting bad property %d has a good status.",
props[0].property_type);
zassert_true(ret < 0);
zassert_equal(ret, -ENOTSUP);
}
ZTEST_USER_F(bq27z746, test_get_some_props_failed_returns_failed_prop_count)
@ -68,16 +65,7 @@ ZTEST_USER_F(bq27z746, test_get_some_props_failed_returns_failed_prop_count)
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
zassert_equal(props[0].status, -ENOTSUP, "Getting bad property %d has a good status.",
props[0].property_type);
zassert_equal(props[1].status, -ENOTSUP, "Getting bad property %d has a good status.",
props[1].property_type);
zassert_ok(props[2].status, "Property %d getting %d has a bad status.", 2,
props[2].property_type);
zassert_equal(ret, 2);
zassert_equal(ret, -ENOTSUP);
}
ZTEST_USER_F(bq27z746, test_get_buffer_prop)
@ -91,7 +79,6 @@ ZTEST_USER_F(bq27z746, test_get_buffer_prop)
prop.property_type = FUEL_GAUGE_MANUFACTURER_NAME;
ret = fuel_gauge_get_buffer_prop(fixture->dev, &prop, &mfg_name, sizeof(mfg_name));
zassert_ok(ret);
zassert_ok(prop.status, "Property %d has a bad status.", prop.property_type);
#if CONFIG_EMUL
/* Only test for fixed values in emulation since the real device might be */
/* reprogrammed and respond with different values */
@ -107,7 +94,6 @@ ZTEST_USER_F(bq27z746, test_get_buffer_prop)
prop.property_type = FUEL_GAUGE_DEVICE_NAME;
ret = fuel_gauge_get_buffer_prop(fixture->dev, &prop, &dev_name, sizeof(dev_name));
zassert_ok(ret);
zassert_ok(prop.status, "Property %d has a bad status.", prop.property_type);
#if CONFIG_EMUL
/* Only test for fixed values in emulation since the real device might be */
/* reprogrammed and respond with different values */
@ -122,7 +108,6 @@ ZTEST_USER_F(bq27z746, test_get_buffer_prop)
ret = fuel_gauge_get_buffer_prop(fixture->dev, &prop, &device_chemistry,
sizeof(device_chemistry));
zassert_ok(ret);
zassert_ok(prop.status, "Property %d has a bad status.", prop.property_type);
#if CONFIG_EMUL
/* Only test for fixed values in emulation since the real device might be */
/* reprogrammed and respond with different values */
@ -191,13 +176,7 @@ ZTEST_USER_F(bq27z746, test_get_props__returns_ok)
},
};
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
/* All props shall have a good status */
for (int i = 0; i < ARRAY_SIZE(props); i++) {
zassert_ok(props[i].status, "Property %d getting %d has a bad status.", i,
props[i].property_type);
}
zassert_ok(fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props)));
/* Check properties for valid ranges */
#if CONFIG_EMUL
@ -239,8 +218,6 @@ ZTEST_USER_F(bq27z746, test_get_props__returns_ok)
/* Not testing props[15]. This property is the status and only has only status bits */
zassert_between_inclusive(props[16].value.design_cap, 0, 32767);
#endif
zassert_ok(ret);
}
ZTEST_SUITE(bq27z746, NULL, bq27z746_setup, NULL, NULL, NULL);

31
tests/drivers/fuel_gauge/max17048/src/test_max17048.c
View file

@ -43,13 +43,10 @@ ZTEST_USER_F(max17048, test_get_all_props_failed_returns_negative)
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
zassert_equal(props[0].status, -ENOTSUP, "Getting bad property %d has a good status.",
props[0].property_type);
zassert_true(ret < 0);
zassert_equal(-ENOTSUP, ret);
}
ZTEST_USER_F(max17048, test_get_some_props_failed_returns_failed_prop_count)
ZTEST_USER_F(max17048, test_get_some_props_failed_returns_errno)
{
struct fuel_gauge_property props[] = {
{
@ -69,16 +66,7 @@ ZTEST_USER_F(max17048, test_get_some_props_failed_returns_failed_prop_count)
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
zassert_equal(props[0].status, -ENOTSUP, "Getting bad property %d has a good status.",
props[0].property_type);
zassert_equal(props[1].status, -ENOTSUP, "Getting bad property %d has a good status.",
props[1].property_type);
zassert_ok(props[2].status, "Property %d getting %d has a bad status.", 2,
props[2].property_type);
zassert_equal(ret, 2);
zassert_equal(ret, -ENOTSUP);
}
@ -101,14 +89,7 @@ ZTEST_USER_F(max17048, test_get_props__returns_ok)
}
};
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
for (int i = 0; i < ARRAY_SIZE(props); i++) {
zassert_ok(props[i].status, "Property %d getting %d has a bad status.", i,
props[i].property_type);
}
zassert_ok(ret);
zassert_ok(fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props)));
}
ZTEST_USER_F(max17048, test_current_rate_zero)
@ -130,10 +111,6 @@ ZTEST_USER_F(max17048, test_current_rate_zero)
emul_max17048_set_crate_status(0);
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
for (int i = 0; i < ARRAY_SIZE(props); i++) {
zassert_ok(props[i].status, "Property %d getting %d has a bad status.", i,
props[i].property_type);
}
zassert_equal(props[0].value.runtime_to_empty, 0,
"Runtime to empty is %d but it should be 0.",
props[0].value.runtime_to_full

91
tests/drivers/fuel_gauge/sbs_gauge/src/test_sbs_gauge.c
View file

@ -32,24 +32,7 @@ static void *sbs_gauge_new_api_setup(void)
return &fixture;
}
ZTEST_USER_F(sbs_gauge_new_api, test_get_all_props_failed_returns_negative)
{
struct fuel_gauge_property props[] = {
{
/* Invalid property */
.property_type = FUEL_GAUGE_PROP_MAX,
},
};
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
zassert_equal(props[0].status, -ENOTSUP, "Getting bad property %d has a good status.",
props[0].property_type);
zassert_true(ret < 0);
}
ZTEST_USER_F(sbs_gauge_new_api, test_get_some_props_failed_returns_failed_prop_count)
ZTEST_USER_F(sbs_gauge_new_api, test_get_some_props_failed_returns_bad_status)
{
struct fuel_gauge_property props[] = {
{
@ -69,19 +52,10 @@ ZTEST_USER_F(sbs_gauge_new_api, test_get_some_props_failed_returns_failed_prop_c
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
zassert_equal(props[0].status, -ENOTSUP, "Getting bad property %d has a good status.",
props[0].property_type);
zassert_equal(props[1].status, -ENOTSUP, "Getting bad property %d has a good status.",
props[1].property_type);
zassert_ok(props[2].status, "Property %d getting %d has a bad status.", 2,
props[2].property_type);
zassert_equal(ret, 2);
zassert_equal(ret, -ENOTSUP, "Getting bad property has a good status.");
}
ZTEST_USER_F(sbs_gauge_new_api, test_set_all_props_failed_returns_negative)
ZTEST_USER_F(sbs_gauge_new_api, test_set_all_props_failed_returns_err)
{
struct fuel_gauge_property props[] = {
{
@ -92,13 +66,10 @@ ZTEST_USER_F(sbs_gauge_new_api, test_set_all_props_failed_returns_negative)
int ret = fuel_gauge_set_props(fixture->dev, props, ARRAY_SIZE(props));
zassert_equal(props[0].status, -ENOTSUP, "Setting bad property %d has a good status.",
props[0].property_type);
zassert_true(ret < 0);
zassert_equal(ret, -ENOTSUP);
}
ZTEST_USER_F(sbs_gauge_new_api, test_set_some_props_failed_returns_failed_prop_count)
ZTEST_USER_F(sbs_gauge_new_api, test_set_some_props_failed_returns_err)
{
struct fuel_gauge_property props[] = {
{
@ -120,16 +91,7 @@ ZTEST_USER_F(sbs_gauge_new_api, test_set_some_props_failed_returns_failed_prop_c
int ret = fuel_gauge_set_props(fixture->dev, props, ARRAY_SIZE(props));
zassert_equal(props[0].status, -ENOTSUP, "Setting bad property %d has a good status.",
props[0].property_type);
zassert_equal(props[1].status, -ENOTSUP, "Setting bad property %d has a good status.",
props[1].property_type);
zassert_ok(props[2].status, "Property %d setting %d has a bad status.", 2,
props[2].property_type);
zassert_equal(ret, 2);
zassert_equal(ret, -ENOTSUP);
}
ZTEST_USER_F(sbs_gauge_new_api, test_set_prop_can_be_get)
@ -179,16 +141,8 @@ ZTEST_USER_F(sbs_gauge_new_api, test_set_prop_can_be_get)
};
zassert_ok(fuel_gauge_set_props(fixture->dev, set_props, ARRAY_SIZE(set_props)));
for (int i = 0; i < ARRAY_SIZE(set_props); i++) {
zassert_ok(set_props[i].status, "Property %d writing %d has a bad status.", i,
set_props[i].property_type);
}
zassert_ok(fuel_gauge_get_props(fixture->dev, get_props, ARRAY_SIZE(get_props)));
for (int i = 0; i < ARRAY_SIZE(get_props); i++) {
zassert_ok(get_props[i].status, "Property %d getting %d has a bad status.", i,
get_props[i].property_type);
}
zassert_equal(get_props[0].value.sbs_mfr_access_word, word);
zassert_equal(get_props[1].value.sbs_remaining_capacity_alarm, word);
@ -276,14 +230,7 @@ ZTEST_USER_F(sbs_gauge_new_api, test_get_props__returns_ok)
},
};
int ret = fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props));
for (int i = 0; i < ARRAY_SIZE(props); i++) {
zassert_ok(props[i].status, "Property %d getting %d has a bad status.", i,
props[i].property_type);
}
zassert_ok(ret);
zassert_ok(fuel_gauge_get_props(fixture->dev, props, ARRAY_SIZE(props)));
}
ZTEST_USER_F(sbs_gauge_new_api, test_set_props__returns_ok)
@ -308,14 +255,7 @@ ZTEST_USER_F(sbs_gauge_new_api, test_set_props__returns_ok)
},
};
int ret = fuel_gauge_set_props(fixture->dev, props, ARRAY_SIZE(props));
for (int i = 0; i < ARRAY_SIZE(props); i++) {
zassert_ok(props[i].status, "Property %d writing %d has a bad status.", i,
props[i].property_type);
}
zassert_ok(ret);
zassert_ok(fuel_gauge_set_props(fixture->dev, props, ARRAY_SIZE(props)));
}
@ -326,22 +266,15 @@ ZTEST_USER_F(sbs_gauge_new_api, test_get_buffer_props__returns_ok)
struct sbs_gauge_manufacturer_name mfg_name;
struct sbs_gauge_device_name dev_name;
struct sbs_gauge_device_chemistry chem;
int ret;
prop.property_type = FUEL_GAUGE_MANUFACTURER_NAME;
ret = fuel_gauge_get_buffer_prop(fixture->dev, &prop, &mfg_name, sizeof(mfg_name));
zassert_ok(prop.status, "Property %d has a bad status.", prop.property_type);
zassert_ok(ret);
zassert_ok(fuel_gauge_get_buffer_prop(fixture->dev, &prop, &mfg_name, sizeof(mfg_name)));
prop.property_type = FUEL_GAUGE_DEVICE_NAME;
ret = fuel_gauge_get_buffer_prop(fixture->dev, &prop, &dev_name, sizeof(dev_name));
zassert_ok(prop.status, "Property %d has a bad status.", prop.property_type);
zassert_ok(ret);
zassert_ok(fuel_gauge_get_buffer_prop(fixture->dev, &prop, &dev_name, sizeof(dev_name)));
prop.property_type = FUEL_GAUGE_DEVICE_CHEMISTRY;
ret = fuel_gauge_get_buffer_prop(fixture->dev, &prop, &chem, sizeof(chem));
zassert_ok(prop.status, "Property %d has a bad status.", prop.property_type);
zassert_ok(ret);
zassert_ok(fuel_gauge_get_buffer_prop(fixture->dev, &prop, &chem, sizeof(chem)));
}
ZTEST_USER_F(sbs_gauge_new_api, test_charging_5v_3a)
@ -362,11 +295,9 @@ ZTEST_USER_F(sbs_gauge_new_api, test_charging_5v_3a)
zassert_ok(fuel_gauge_get_prop(fixture->dev, &voltage));
zassert_ok(fuel_gauge_get_prop(fixture->dev, &current));
zassert_ok(voltage.status);
zassert_equal(voltage.value.voltage, expected_uV, "Got %d instead of %d",
voltage.value.voltage, expected_uV);
zassert_ok(current.status);
zassert_equal(current.value.current, expected_uA, "Got %d instead of %d",
current.value.current, expected_uA);
}