From 2225937b6345a833849aefa3f9a946d00e210554 Mon Sep 17 00:00:00 2001 From: Andreas Fritiofson Date: Sun, 12 Jan 2025 14:24:02 +0100 Subject: [PATCH] renaulttwizy: Fix deprecated operations on volatile Extract the duplicated code to a helper method. Do the flag setting via a local variable to make it explicit when the volatile variable is accessed. The sensors state really should use atomics if it may be accessed from different contexts (threads/interrupts), or otherwise just remove volatile since it's not doing anything useful. --- .../vehicle_renaulttwizy/src/rt_battmon.cpp | 14 ++++++++ .../vehicle_renaulttwizy/src/rt_can.cpp | 33 ++++--------------- .../src/vehicle_renaulttwizy.h | 1 + 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/rt_battmon.cpp b/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/rt_battmon.cpp index e38d2cbb6..12a8c42ac 100644 --- a/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/rt_battmon.cpp +++ b/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/rt_battmon.cpp @@ -249,6 +249,20 @@ void OvmsVehicleRenaultTwizy::BatteryUpdate() } +/** + * BatterySetSensorAndUpdate: Set sensor state and update if all done + */ +void OvmsVehicleRenaultTwizy::BatterySetSensorAndUpdate(uint8_t flag) +{ + uint8_t batt_sensors_state = twizy_batt_sensors_state; + batt_sensors_state |= flag; + twizy_batt_sensors_state = batt_sensors_state; + if ((batt_sensors_state & BATT_SENSORS_READY) >= BATT_SENSORS_GOTALL) { + BatteryUpdate(); + } +} + + /** * BatteryReset: reset deviations, alerts & watches */ diff --git a/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/rt_can.cpp b/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/rt_can.cpp index 8a9a5e575..51f32b9bd 100644 --- a/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/rt_can.cpp +++ b/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/rt_can.cpp @@ -160,7 +160,6 @@ void OvmsVehicleRenaultTwizy::CanResponder(const CAN_frame_t* p_frame) } } - /** * Asynchronous CAN RX handler */ @@ -353,12 +352,8 @@ void OvmsVehicleRenaultTwizy::IncomingFrameCan1(CAN_frame_t* p_frame) } } } - - // detect fetch completion: - twizy_batt_sensors_state |= BATT_SENSORS_GOT554; - if ((twizy_batt_sensors_state & BATT_SENSORS_READY) >= BATT_SENSORS_GOTALL) { - BatteryUpdate(); - } + + BatterySetSensorAndUpdate(BATT_SENSORS_GOT554); } break; @@ -409,11 +404,7 @@ void OvmsVehicleRenaultTwizy::IncomingFrameCan1(CAN_frame_t* p_frame) twizy_cell[8].volt_new = ((UINT) CAN_NIBL(4) << 8) | ((UINT) CAN_BYTE(5)); twizy_cell[9].volt_new = ((UINT) CAN_BYTE(6) << 4) | ((UINT) CAN_NIBH(7)); - // detect fetch completion: - twizy_batt_sensors_state |= BATT_SENSORS_GOT557; - if ((twizy_batt_sensors_state & BATT_SENSORS_READY) >= BATT_SENSORS_GOTALL) { - BatteryUpdate(); - } + BatterySetSensorAndUpdate(BATT_SENSORS_GOT557); } break; @@ -428,11 +419,7 @@ void OvmsVehicleRenaultTwizy::IncomingFrameCan1(CAN_frame_t* p_frame) twizy_cell[12].volt_new = ((UINT) CAN_BYTE(3) << 4) | ((UINT) CAN_NIBH(4)); twizy_cell[13].volt_new = ((UINT) CAN_NIBL(4) << 8) | ((UINT) CAN_BYTE(5)); - // detect fetch completion: - twizy_batt_sensors_state |= BATT_SENSORS_GOT55E; - if ((twizy_batt_sensors_state & BATT_SENSORS_READY) >= BATT_SENSORS_GOTALL) { - BatteryUpdate(); - } + BatterySetSensorAndUpdate(BATT_SENSORS_GOT55E); } break; @@ -449,11 +436,7 @@ void OvmsVehicleRenaultTwizy::IncomingFrameCan1(CAN_frame_t* p_frame) v2 = ((UINT) CAN_NIBL(6) << 8) | ((UINT) CAN_BYTE(7)); twizy_batt[0].volt_new = (v1 + v2 + 1) >> 1; - // detect fetch completion: - twizy_batt_sensors_state |= BATT_SENSORS_GOT55F; - if ((twizy_batt_sensors_state & BATT_SENSORS_READY) >= BATT_SENSORS_GOTALL) { - BatteryUpdate(); - } + BatterySetSensorAndUpdate(BATT_SENSORS_GOT55F); } break; @@ -757,11 +740,7 @@ void OvmsVehicleRenaultTwizy::IncomingFrameCan1(CAN_frame_t* p_frame) } } - // detect fetch completion: - twizy_batt_sensors_state |= BATT_SENSORS_GOT700; - if ((twizy_batt_sensors_state & BATT_SENSORS_READY) >= BATT_SENSORS_GOTALL) { - BatteryUpdate(); - } + BatterySetSensorAndUpdate(BATT_SENSORS_GOT700); } break; diff --git a/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/vehicle_renaulttwizy.h b/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/vehicle_renaulttwizy.h index 8486c0aca..174bac47d 100644 --- a/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/vehicle_renaulttwizy.h +++ b/vehicle/OVMS.V3/components/vehicle_renaulttwizy/src/vehicle_renaulttwizy.h @@ -374,6 +374,7 @@ class OvmsVehicleRenaultTwizy : public OvmsVehicle void BatteryUpdate(); void BatteryReset(); void BatterySendDataUpdate(bool force = false); + void BatterySetSensorAndUpdate(uint8_t flag); vehicle_command_t CommandBatt(int verbosity, OvmsWriter* writer, OvmsCommand* cmd, int argc, const char* const* argv); protected: