Skip to content

AP_Periph: Use battery SOC only when available#32105

Open
lgarciaos wants to merge 1 commit intoArduPilot:masterfrom
lgarciaos:apperiph-soc-flag
Open

AP_Periph: Use battery SOC only when available#32105
lgarciaos wants to merge 1 commit intoArduPilot:masterfrom
lgarciaos:apperiph-soc-flag

Conversation

@lgarciaos
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lgarciaos
Copy link
Copy Markdown
Contributor Author

Fixed typo

Comment on lines +129 to +130
_interim_state.state_of_charge_pct = msg.state_of_charge_pct;
_interim_state.has_state_of_charge_pct = true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These don't seem to exist?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @IamPete1, I removed those lines and now I think it makes sense to me. SoC is already handled as it should.

pkt.state_of_health_pct = state_of_health_pct;
}

pkt.state_of_charge_pct = UAVCAN_EQUIPMENT_POWER_BATTERYINFO_STATE_OF_CHARGE_UNKNOWN;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just do this?

Suggested change
pkt.state_of_charge_pct = UAVCAN_EQUIPMENT_POWER_BATTERYINFO_STATE_OF_CHARGE_UNKNOWN;
uint8_t percentage = UAVCAN_EQUIPMENT_POWER_BATTERYINFO_STATE_OF_CHARGE_UNKNOWN;
if (battery_lib.capacity_remaining_pct(percentage, i)) {
pkt.state_of_charge_pct = percentage;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh geez, I didn't mean that. No one look at that embarrassing suggestion! LOL. I meant this.. I SWEAR

Suggested change
pkt.state_of_charge_pct = UAVCAN_EQUIPMENT_POWER_BATTERYINFO_STATE_OF_CHARGE_UNKNOWN;
uint8_t percentage = 0;
if (battery_lib.capacity_remaining_pct(percentage, i)) {
pkt.state_of_charge_pct = percentage;
} else {
pkt.state_of_charge_pct = UAVCAN_EQUIPMENT_POWER_BATTERYINFO_STATE_OF_CHARGE_UNKNOWN;
}

or even a one-liner:

Suggested change
pkt.state_of_charge_pct = UAVCAN_EQUIPMENT_POWER_BATTERYINFO_STATE_OF_CHARGE_UNKNOWN;
pkt.state_of_charge_pct = battery_lib.capacity_remaining_pct(percentage, i) ? percentage : UAVCAN_EQUIPMENT_POWER_BATTERYINFO_STATE_OF_CHARGE_UNKNOWN;

Just trying to avoid the unnecessary double assignment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants