Avoid resetting charge_max_level during CHG_LIMIT_GET_LIMIT #7
Conversation
board/hx20/battery.c
Outdated
| @@ -363,8 +363,9 @@ static enum ec_status cmd_charging_limit_control(struct host_cmd_handler_args *a | |||
| charging_maximum_level = charging_maximum_level | CHG_LIMIT_OVERRIDE; | |||
|
|
|||
| if (p->modes & CHG_LIMIT_GET_LIMIT) { | |||
| system_get_bbram(SYSTEM_BBRAM_IDX_CHG_MAX, &charging_maximum_level); | |||
| r->max_percentage = charging_maximum_level; | |||
| uint8_t max = 0; | |||
There was a problem hiding this comment.
fwiw I think the code style here is to declare all variables at the beginning of the function
There was a problem hiding this comment.
Fair. I was iffy on whether I wanted to introduce a temporary anyway, given that ec_response_chg_limit_control::max_percentage is itself a uint8_t. I switched over to reading directly into it.
This may cause a compat problem if the response from this command ever changes, but the correct way to handle that would be with a new command version anyway . . . 😄
Thanks!
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
I'm not certain about the "labeler" failure: |
|
Moved this over to |
DHowett
changed the title
Reading into
charging_maximum_leveloverwritesCHG_LIMIT_OVERRIDE;GET_LIMITshould not change global state.Fixes #6.