AP_BattMonitor: Remove double cast in mah calculation #19540
Conversation
| @@ -51,6 +51,8 @@ class AP_BattMonitor_UAVCAN : public AP_BattMonitor_Backend | |||
| static void handle_battery_info_aux_trampoline(AP_UAVCAN* ap_uavcan, uint8_t node_id, const BattInfoAuxCb &cb); | |||
| static void handle_mppt_stream_trampoline(AP_UAVCAN* ap_uavcan, uint8_t node_id, const MpptStreamCb &cb); | |||
|
|
|||
| static float calculate_mah(float amps, float dt) { return (float) (amps * dt * 0.0000002778f); } | |||
There was a problem hiding this comment.
Needs a comment to say where the magic constant came from, and arguably should actually be a helper that is in the general battery backend (or a seperate static function that any of the backends could include as needed), since the analog monitor has the same code path and we should be trying to hold these to do identical calculations.
There was a problem hiding this comment.
Alright, I made it a static function in the backend header. Thought about virtual but it would make it harder to run the test class, although its currently causing errors for some reason it might just be best to scrap it.
I also made Analog use the new function.
|
Really nicely done. @peterbarker should probably decide if he's happy with the test code. |
|
Looking good! Two things:
If you need help with the squashing let me know? |
arBalasquide
force-pushed
the
pr-remove-double-battuavcan
branch
from
3b18e01
to
d47c1ef
Compare
|
@hendjoshsr71 Thank you for the suggestions. I've squashed the commits now let me know if its OK. |
|
just needs intel copyright removed |
arBalasquide
force-pushed
the
pr-remove-double-battuavcan
branch
from
d47c1ef
to
66c1e5b
Compare
This PR addresses issue #19452.
Also adds regression tests and accuracy tests to validate there isn't any loss of accuracy from removing the double casts.