Skip to content

Commit 8bfc57f

Browse files
committed
fix: prevent stale HA values from overwriting writable entities
HA's MQTT Number entity defaulted to optimistic mode in the discovery payload, so HA treated its locally cached value as authoritative and republished it as a command on restart - the gateway then forwarded that to the vehicle, resetting target SoC and total battery capacity (#375). The same gap existed in select and text discovery payloads. Mark every writable discovery payload (number, select, text) with `optimistic: false` so HA reads state from `state_topic`, and route number-entity commands through an eager-echo + rollback path in the command dispatcher: capture the prior value, echo the new value to the state topic on receipt for instant UX feedback, and republish the prior value on failure so the UI reflects rejections. Closes #375
1 parent d93b39f commit 8bfc57f

8 files changed

Lines changed: 425 additions & 43 deletions

File tree

‎src/handlers/command/base.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,27 @@ def topic(cls) -> str:
5353
async def handle(self, payload: str) -> CommandProcessingResult:
5454
raise NotImplementedError
5555

56+
def echo_state_topic(self) -> str | None:
57+
"""State topic for eager echo and rollback.
58+
59+
Return None to disable eager echo (default).
60+
"""
61+
return None
62+
63+
def capture_current_state(self) -> object | None:
64+
"""Snapshot the current state value for rollback on failure.
65+
66+
Return None to skip the rollback publish.
67+
"""
68+
return None
69+
70+
def echo_payload(self, raw_payload: str) -> object | None:
71+
"""Convert the raw MQTT command payload to a state-topic echo value.
72+
73+
Return None to skip the eager publish for this call (e.g. invalid payload).
74+
"""
75+
return raw_payload
76+
5677
@property
5778
def saic_api(self) -> SaicApi:
5879
return self.__saic_api

‎src/handlers/command/drivetrain/drivetrain_soc_target.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,24 @@ class DrivetrainSoCTargetCommand(PayloadConvertingCommandHandler[TargetBatteryCo
2222
def topic(cls) -> str:
2323
return mqtt_topics.DRIVETRAIN_SOC_TARGET_SET
2424

25+
@override
26+
def echo_state_topic(self) -> str:
27+
return mqtt_topics.DRIVETRAIN_SOC_TARGET
28+
29+
@override
30+
def capture_current_state(self) -> int | None:
31+
target = self.vehicle_state.target_soc
32+
return None if target is None else target.percentage
33+
34+
@override
35+
def echo_payload(self, raw_payload: str) -> int | None:
36+
try:
37+
return TargetBatteryCode.from_percentage(
38+
int(raw_payload.strip())
39+
).percentage
40+
except (ValueError, KeyError):
41+
return None
42+
2543
@staticmethod
2644
@override
2745
def convert_payload(payload: str) -> TargetBatteryCode:

‎src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,21 @@ class DrivetrainTotalBatteryCapacitySetCommand(FloatCommandHandler):
1919
def topic(cls) -> str:
2020
return mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY_SET
2121

22+
@override
23+
def echo_state_topic(self) -> str:
24+
return mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY
25+
26+
@override
27+
def capture_current_state(self) -> float | None:
28+
return self.vehicle_state.vehicle.custom_battery_capacity
29+
30+
@override
31+
def echo_payload(self, raw_payload: str) -> float | None:
32+
try:
33+
return float(raw_payload.strip())
34+
except ValueError:
35+
return None
36+
2237
@override
2338
async def handle_typed_payload(self, payload: float) -> CommandProcessingResult:
2439
LOG.info("Setting Total Battery Capacity to %f", payload)

‎src/handlers/command/gateway/refresh_period.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,32 @@
1010
import mqtt_topics
1111

1212

13+
def _safe_int_payload(raw_payload: str) -> int | None:
14+
try:
15+
return int(raw_payload.strip())
16+
except ValueError:
17+
return None
18+
19+
1320
class RefreshPeriodActiveCommand(IntCommandHandler):
1421
@classmethod
1522
@override
1623
def topic(cls) -> str:
1724
return mqtt_topics.REFRESH_PERIOD_ACTIVE_SET
1825

26+
@override
27+
def echo_state_topic(self) -> str:
28+
return mqtt_topics.REFRESH_PERIOD_ACTIVE
29+
30+
@override
31+
def capture_current_state(self) -> int | None:
32+
value = self.vehicle_state.refresh_period_active
33+
return None if value < 0 else value
34+
35+
@override
36+
def echo_payload(self, raw_payload: str) -> int | None:
37+
return _safe_int_payload(raw_payload)
38+
1939
@override
2040
async def handle_typed_payload(self, payload: int) -> CommandProcessingResult:
2141
self.vehicle_state.set_refresh_period_active(payload)
@@ -28,6 +48,19 @@ class RefreshPeriodInactiveCommand(IntCommandHandler):
2848
def topic(cls) -> str:
2949
return mqtt_topics.REFRESH_PERIOD_INACTIVE_SET
3050

51+
@override
52+
def echo_state_topic(self) -> str:
53+
return mqtt_topics.REFRESH_PERIOD_INACTIVE
54+
55+
@override
56+
def capture_current_state(self) -> int | None:
57+
value = self.vehicle_state.refresh_period_inactive
58+
return None if value < 0 else value
59+
60+
@override
61+
def echo_payload(self, raw_payload: str) -> int | None:
62+
return _safe_int_payload(raw_payload)
63+
3164
@override
3265
async def handle_typed_payload(self, payload: int) -> CommandProcessingResult:
3366
self.vehicle_state.set_refresh_period_inactive(payload)
@@ -40,6 +73,19 @@ class RefreshPeriodInactiveGraceCommand(IntCommandHandler):
4073
def topic(cls) -> str:
4174
return mqtt_topics.REFRESH_PERIOD_INACTIVE_GRACE_SET
4275

76+
@override
77+
def echo_state_topic(self) -> str:
78+
return mqtt_topics.REFRESH_PERIOD_INACTIVE_GRACE
79+
80+
@override
81+
def capture_current_state(self) -> int | None:
82+
value = self.vehicle_state.refresh_period_inactive_grace
83+
return None if value < 0 else value
84+
85+
@override
86+
def echo_payload(self, raw_payload: str) -> int | None:
87+
return _safe_int_payload(raw_payload)
88+
4389
@override
4490
async def handle_typed_payload(self, payload: int) -> CommandProcessingResult:
4591
self.vehicle_state.set_refresh_period_inactive_grace(payload)
@@ -52,6 +98,19 @@ class RefreshPeriodAfterShutdownCommand(IntCommandHandler):
5298
def topic(cls) -> str:
5399
return mqtt_topics.REFRESH_PERIOD_AFTER_SHUTDOWN_SET
54100

101+
@override
102+
def echo_state_topic(self) -> str:
103+
return mqtt_topics.REFRESH_PERIOD_AFTER_SHUTDOWN
104+
105+
@override
106+
def capture_current_state(self) -> int | None:
107+
value = self.vehicle_state.refresh_period_after_shutdown
108+
return None if value < 0 else value
109+
110+
@override
111+
def echo_payload(self, raw_payload: str) -> int | None:
112+
return _safe_int_payload(raw_payload)
113+
55114
@override
56115
async def handle_typed_payload(self, payload: int) -> CommandProcessingResult:
57116
self.vehicle_state.set_refresh_period_after_shutdown(payload)

‎src/handlers/vehicle_command.py‎

Lines changed: 97 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,26 @@ def __init__(
5757
def publisher(self) -> Publisher:
5858
return self.vehicle_state.publisher
5959

60+
def __publish_state_echo(self, *, topic: str, value: object | None) -> None:
61+
if value is None:
62+
return
63+
full_topic = self.vehicle_state.get_topic(topic)
64+
try:
65+
if isinstance(value, bool):
66+
self.publisher.publish_bool(full_topic, value)
67+
elif isinstance(value, int):
68+
self.publisher.publish_int(full_topic, value)
69+
elif isinstance(value, float):
70+
self.publisher.publish_float(full_topic, value)
71+
else:
72+
self.publisher.publish_str(full_topic, str(value))
73+
except Exception:
74+
LOG.warning(
75+
"Failed to publish state echo for topic %s",
76+
full_topic,
77+
exc_info=True,
78+
)
79+
6080
def __report_command_failure(
6181
self,
6282
*,
@@ -106,6 +126,23 @@ async def handle_mqtt_command(self, *, topic: str, payload: str) -> None:
106126
handler=handler, payload=payload, analyzed_topic=analyzed_topic
107127
)
108128

129+
async def __run_handler_and_report_success(
130+
self,
131+
*,
132+
handler: CommandHandlerBase,
133+
payload: str,
134+
analyzed_topic: _MqttCommandTopic,
135+
) -> None:
136+
execution_result = await handler.handle(payload)
137+
self.publisher.publish_str(analyzed_topic.response_no_global, "Success")
138+
if execution_result.force_refresh:
139+
self.vehicle_state.set_refresh_mode(
140+
RefreshMode.FORCE,
141+
f"after command execution on topic {analyzed_topic.command_no_vin}",
142+
)
143+
if execution_result.clear_command:
144+
self.publisher.clear_topic(analyzed_topic.command_no_global)
145+
109146
async def __execute_mqtt_command_handler(
110147
self,
111148
*,
@@ -114,65 +151,87 @@ async def __execute_mqtt_command_handler(
114151
analyzed_topic: _MqttCommandTopic,
115152
) -> None:
116153
topic = analyzed_topic.command_no_vin
117-
topic_no_global = analyzed_topic.command_no_global
118154
result_topic = analyzed_topic.response_no_global
119155

156+
echo_topic = handler.echo_state_topic()
157+
prior_state: object | None = None
158+
echoed = False
159+
if echo_topic is not None:
160+
prior_state = handler.capture_current_state()
161+
echoed_value = handler.echo_payload(payload)
162+
if echoed_value is not None:
163+
self.__publish_state_echo(topic=echo_topic, value=echoed_value)
164+
echoed = True
165+
166+
def rollback_state_echo() -> None:
167+
if echoed and echo_topic is not None and prior_state is not None:
168+
self.__publish_state_echo(topic=echo_topic, value=prior_state)
169+
120170
try:
121-
execution_result = await handler.handle(payload)
122-
self.publisher.publish_str(result_topic, "Success")
123-
if execution_result.force_refresh:
124-
self.vehicle_state.set_refresh_mode(
125-
RefreshMode.FORCE, f"after command execution on topic {topic}"
126-
)
127-
if execution_result.clear_command:
128-
self.publisher.clear_topic(topic_no_global)
171+
await self.__run_handler_and_report_success(
172+
handler=handler, payload=payload, analyzed_topic=analyzed_topic
173+
)
129174
except MqttGatewayException as e:
175+
rollback_state_echo()
130176
self.__report_command_failure(
131177
command=topic, result_topic=result_topic, detail=e.message, exc=e
132178
)
133179
except SaicLogoutException:
134-
LOG.warning(
135-
"API Client was logged out, attempting immediate relogin and retry"
180+
await self.__handle_logout_and_retry(
181+
handler=handler,
182+
payload=payload,
183+
analyzed_topic=analyzed_topic,
184+
rollback_state_echo=rollback_state_echo,
136185
)
137-
try:
138-
await self.relogin_handler.force_login()
139-
except Exception as login_err:
140-
self.__report_command_failure(
141-
command=topic,
142-
result_topic=result_topic,
143-
detail=f"relogin failed ({login_err})",
144-
exc=login_err,
145-
)
146-
return
147-
try:
148-
execution_result = await handler.handle(payload)
149-
self.publisher.publish_str(result_topic, "Success")
150-
if execution_result.force_refresh:
151-
self.vehicle_state.set_refresh_mode(
152-
RefreshMode.FORCE,
153-
f"after command execution on topic {topic}",
154-
)
155-
if execution_result.clear_command:
156-
self.publisher.clear_topic(topic_no_global)
157-
except Exception as retry_err:
158-
self.__report_command_failure(
159-
command=topic,
160-
result_topic=result_topic,
161-
detail=str(retry_err),
162-
exc=retry_err,
163-
)
164186
except SaicApiException as se:
187+
rollback_state_echo()
165188
self.__report_command_failure(
166189
command=topic, result_topic=result_topic, detail=se.message, exc=se
167190
)
168191
except Exception as e:
192+
rollback_state_echo()
169193
self.__report_command_failure(
170194
command=topic,
171195
result_topic=result_topic,
172196
detail="unexpected error",
173197
exc=e,
174198
)
175199

200+
async def __handle_logout_and_retry(
201+
self,
202+
*,
203+
handler: CommandHandlerBase,
204+
payload: str,
205+
analyzed_topic: _MqttCommandTopic,
206+
rollback_state_echo: Callable[[], None],
207+
) -> None:
208+
topic = analyzed_topic.command_no_vin
209+
result_topic = analyzed_topic.response_no_global
210+
LOG.warning("API Client was logged out, attempting immediate relogin and retry")
211+
try:
212+
await self.relogin_handler.force_login()
213+
except Exception as login_err:
214+
rollback_state_echo()
215+
self.__report_command_failure(
216+
command=topic,
217+
result_topic=result_topic,
218+
detail=f"relogin failed ({login_err})",
219+
exc=login_err,
220+
)
221+
return
222+
try:
223+
await self.__run_handler_and_report_success(
224+
handler=handler, payload=payload, analyzed_topic=analyzed_topic
225+
)
226+
except Exception as retry_err:
227+
rollback_state_echo()
228+
self.__report_command_failure(
229+
command=topic,
230+
result_topic=result_topic,
231+
detail=str(retry_err),
232+
exc=retry_err,
233+
)
234+
176235
def __get_command_topics(self, topic: str) -> _MqttCommandTopic:
177236
global_topic_removed = topic.removeprefix(self.global_mqtt_topic).removeprefix(
178237
"/"

‎src/integrations/home_assistant/base.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ def _publish_select(
5555
"value_template": value_template,
5656
"command_template": command_template,
5757
"options": options,
58+
"optimistic": False,
5859
"enabled_by_default": enabled,
5960
}
6061
if entity_category is not None:
@@ -86,6 +87,7 @@ def _publish_text(
8687
"value_template": value_template,
8788
"command_template": command_template,
8889
"retain": str(retain).lower(),
90+
"optimistic": False,
8991
"enabled_by_default": enabled,
9092
}
9193
if min_value is not None:
@@ -152,6 +154,7 @@ def _publish_number(
152154
"command_topic": self._get_command_topic(topic),
153155
"value_template": value_template,
154156
"retain": str(retain).lower(),
157+
"optimistic": False,
155158
"mode": mode,
156159
"min": min_value,
157160
"max": max_value,

0 commit comments

Comments
 (0)