Skip to content

Commit 26e1584

Browse files
Bre77Bre77
authored
fix(iobroker): reconcile rejected vehicle/energy writes instead of leaving state looking applied (#124)
handleStateChange() in VehicleHandler/EnergyHandler used to catch and log its own write failures, so the onStateChange() caller in main.ts never saw the rejection and the object state kept showing the requested value as if it had applied. Writes now go through a writeAndReconcile() helper that acks the requested value on success and restores the last confirmed value on failure, letting the failure propagate to onStateChange() as the single place that logs it.
1 parent 6a555ae commit 26e1584

6 files changed

Lines changed: 256 additions & 140 deletions

File tree

‎.changeset/iob-write-reconcile.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"iobroker.teslemetry": patch
3+
---
4+
5+
Reconcile rejected vehicle/energy writes instead of leaving the object state showing the requested value as applied: `VehicleHandler`/`EnergyHandler` now let write failures propagate to `onStateChange()` (the single place that logs and reports the error) and restore the last confirmed value on the failed state, while a successful write now explicitly acks the new value.

‎AGENTS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ pnpm link --global n8n-nodes-teslemetry
168168

169169
**Gotcha**: the `HvacLeft`/`HvacRightTemperatureRequest` SSE signals and `TeslemetryVehicleApi.setTemps()`'s positional args are physical left/right seats, not driver/passenger - on RHD vehicles the driver sits on the right. `StateManager` stores each vehicle's `config.rhd` (from `VehicleDetails.metadata`, passed in at `createVehicleStates()`) and exposes it via `isRhd(vin)`, which both the SSE mapping in `updateVehicleDataFromSignals` and the `setTemps()` write in `VehicleHandler.handleStateChange` consult to pick the correct side - mirrors the Homebridge plugin's `ClimateService.isRHD` pattern.
170170

171+
**Gotcha**: `VehicleHandler`/`EnergyHandler.handleStateChange()` don't catch their own write failures - a rejected SDK write propagates up to `main.ts`'s `onStateChange()`, the single place that logs it, so a new write branch must not add its own catch/log or the failure logs twice. Each write goes through `writeAndReconcile(id, value, write)` (private to each handler), which acks the requested value on success and re-acks the last confirmed value on failure, so a rejected command never leaves the ioBroker object state looking like it applied.
172+
171173
**Gotcha**: changesets bumps `package.json`/`CHANGELOG.md` on release but never touches `io-package.json` - its `common.version` and `common.news` need a manual sync on every release or the ioBroker repochecker hard-fails submission to `ioBroker.repositories`. No Teslemetry brand/logo asset lives in this monorepo; the real logo mark lives in the separate `website3` repo (its `public/web-app-manifest-512x512.png` is the highest-res copy) - source icons from there, don't hand-draw a placeholder.
172174

173175
## Technology Stack

‎packages/iobroker.teslemetry/lib/EnergyHandler.ts‎

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@ export class EnergyHandler {
2020
this.adapter.log.info(`Registered energy site: ${siteId}`);
2121
}
2222

23+
/**
24+
* Runs a write against the energy site, acking the requested value on success.
25+
* A rejected write restores the last confirmed value so the object state never
26+
* shows a requested value that was never actually applied.
27+
*/
28+
private async writeAndReconcile(id: string, value: any, write: () => Promise<any>): Promise<void> {
29+
const prior = await this.adapter.getStateAsync(id);
30+
try {
31+
await write();
32+
} catch (error) {
33+
await this.adapter.setStateAsync(id, prior?.val ?? null, true);
34+
throw error;
35+
}
36+
await this.adapter.setStateAsync(id, value, true);
37+
}
38+
2339
/**
2440
* Execute an energy site command
2541
*/
@@ -32,21 +48,16 @@ export class EnergyHandler {
3248

3349
this.adapter.log.debug(`Executing command ${command} for energy site ${siteId}`);
3450

35-
try {
36-
switch (command) {
37-
case 'storm_mode':
38-
if (params?.enabled !== undefined) {
39-
await site.setStormMode(params.enabled);
40-
this.adapter.log.info(`Set storm mode to ${params.enabled} for site ${siteId}`);
41-
}
42-
break;
51+
switch (command) {
52+
case 'storm_mode':
53+
if (params?.enabled !== undefined) {
54+
await site.setStormMode(params.enabled);
55+
this.adapter.log.info(`Set storm mode to ${params.enabled} for site ${siteId}`);
56+
}
57+
break;
4358

44-
default:
45-
this.adapter.log.warn(`Unknown command: ${command}`);
46-
}
47-
} catch (error: any) {
48-
this.adapter.log.error(`Error executing command ${command} for site ${siteId}: ${error.message}`);
49-
throw error;
59+
default:
60+
this.adapter.log.warn(`Unknown command: ${command}`);
5061
}
5162
}
5263

@@ -60,30 +71,28 @@ export class EnergyHandler {
6071
return;
6172
}
6273

63-
try {
64-
// Handle commands
65-
if (category === 'commands') {
66-
if (stateName === 'storm_mode') {
67-
await this.executeCommand(siteId, 'storm_mode', { enabled: value });
68-
}
69-
return;
74+
// Handle commands
75+
if (category === 'commands') {
76+
if (stateName === 'storm_mode') {
77+
await this.executeCommand(siteId, 'storm_mode', { enabled: value });
7078
}
79+
return;
80+
}
7181

72-
// Handle writable operation states
73-
if (category === 'operation') {
74-
if (stateName === 'mode') {
75-
await site.setOperationMode(value);
76-
this.adapter.log.info(`Set operation mode to ${value} for site ${siteId}`);
77-
} else if (stateName === 'backup_reserve_percent') {
78-
await site.setBackupReserve(value);
79-
this.adapter.log.info(`Set backup reserve to ${value}% for site ${siteId}`);
80-
} else if (stateName === 'off_grid_reserve_percent') {
81-
await site.setOffGridVehicleChargingReserve(value);
82-
this.adapter.log.info(`Set off-grid reserve to ${value}% for site ${siteId}`);
83-
}
82+
// Handle writable operation states
83+
if (category === 'operation') {
84+
if (stateName === 'mode') {
85+
await this.writeAndReconcile(`energy.${siteId}.operation.mode`, value, () => site.setOperationMode(value));
86+
this.adapter.log.info(`Set operation mode to ${value} for site ${siteId}`);
87+
} else if (stateName === 'backup_reserve_percent') {
88+
await this.writeAndReconcile(`energy.${siteId}.operation.backup_reserve_percent`, value, () => site.setBackupReserve(value));
89+
this.adapter.log.info(`Set backup reserve to ${value}% for site ${siteId}`);
90+
} else if (stateName === 'off_grid_reserve_percent') {
91+
await this.writeAndReconcile(`energy.${siteId}.operation.off_grid_reserve_percent`, value, () =>
92+
site.setOffGridVehicleChargingReserve(value)
93+
);
94+
this.adapter.log.info(`Set off-grid reserve to ${value}% for site ${siteId}`);
8495
}
85-
} catch (error: any) {
86-
this.adapter.log.error(`Error handling state change for ${siteId}.${category}.${stateName}: ${error.message}`);
8796
}
8897
}
8998

‎packages/iobroker.teslemetry/lib/VehicleHandler.ts‎

Lines changed: 108 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@ export class VehicleHandler {
2020
this.adapter.log.info(`Registered vehicle: ${vin}`);
2121
}
2222

23+
/**
24+
* Runs a write against the vehicle, acking the requested value on success. A
25+
* rejected write restores the last confirmed value so the object state never
26+
* shows a requested value that was never actually applied.
27+
*/
28+
private async writeAndReconcile(id: string, value: any, write: () => Promise<any>): Promise<void> {
29+
const prior = await this.adapter.getStateAsync(id);
30+
try {
31+
await write();
32+
} catch (error) {
33+
await this.adapter.setStateAsync(id, prior?.val ?? null, true);
34+
throw error;
35+
}
36+
await this.adapter.setStateAsync(id, value, true);
37+
}
38+
2339
/**
2440
* Execute a vehicle command
2541
*/
@@ -32,73 +48,64 @@ export class VehicleHandler {
3248

3349
this.adapter.log.debug(`Executing command ${command} for vehicle ${vin}`);
3450

35-
try {
36-
switch (command) {
37-
case 'wake':
38-
await vehicle.wakeUp();
39-
this.adapter.log.info(`Woke up vehicle ${vin}`);
40-
break;
41-
42-
case 'lock':
43-
await vehicle.lockDoors();
44-
this.adapter.log.info(`Locked vehicle ${vin}`);
45-
await this.adapter.setStateAsync(`vehicles.${vin}.state.locked`, true, true);
46-
break;
47-
48-
case 'unlock':
49-
await vehicle.unlockDoors();
50-
this.adapter.log.info(`Unlocked vehicle ${vin}`);
51-
await this.adapter.setStateAsync(`vehicles.${vin}.state.locked`, false, true);
52-
break;
53-
54-
case 'start_climate':
55-
await vehicle.startAutoConditioning();
56-
this.adapter.log.info(`Started climate for vehicle ${vin}`);
57-
await this.adapter.setStateAsync(`vehicles.${vin}.climate.is_climate_on`, true, true);
58-
break;
59-
60-
case 'stop_climate':
61-
await vehicle.stopAutoConditioning();
62-
this.adapter.log.info(`Stopped climate for vehicle ${vin}`);
63-
await this.adapter.setStateAsync(`vehicles.${vin}.climate.is_climate_on`, false, true);
64-
break;
65-
66-
case 'start_charging':
67-
await vehicle.startCharging();
68-
this.adapter.log.info(`Started charging for vehicle ${vin}`);
69-
break;
70-
71-
case 'stop_charging':
72-
await vehicle.stopCharging();
73-
this.adapter.log.info(`Stopped charging for vehicle ${vin}`);
74-
break;
75-
76-
case 'flash_lights':
77-
await vehicle.flashLights();
78-
this.adapter.log.info(`Flashed lights for vehicle ${vin}`);
79-
break;
80-
81-
case 'honk_horn':
82-
await vehicle.honkHorn();
83-
this.adapter.log.info(`Honked horn for vehicle ${vin}`);
84-
break;
85-
86-
case 'open_frunk':
87-
await vehicle.actuateTrunk('front');
88-
this.adapter.log.info(`Opened frunk for vehicle ${vin}`);
89-
break;
90-
91-
case 'open_trunk':
92-
await vehicle.actuateTrunk('rear');
93-
this.adapter.log.info(`Opened trunk for vehicle ${vin}`);
94-
break;
95-
96-
default:
97-
this.adapter.log.warn(`Unknown command: ${command}`);
98-
}
99-
} catch (error: any) {
100-
this.adapter.log.error(`Error executing command ${command} for vehicle ${vin}: ${error.message}`);
101-
throw error;
51+
switch (command) {
52+
case 'wake':
53+
await vehicle.wakeUp();
54+
this.adapter.log.info(`Woke up vehicle ${vin}`);
55+
break;
56+
57+
case 'lock':
58+
await this.writeAndReconcile(`vehicles.${vin}.state.locked`, true, () => vehicle.lockDoors());
59+
this.adapter.log.info(`Locked vehicle ${vin}`);
60+
break;
61+
62+
case 'unlock':
63+
await this.writeAndReconcile(`vehicles.${vin}.state.locked`, false, () => vehicle.unlockDoors());
64+
this.adapter.log.info(`Unlocked vehicle ${vin}`);
65+
break;
66+
67+
case 'start_climate':
68+
await this.writeAndReconcile(`vehicles.${vin}.climate.is_climate_on`, true, () => vehicle.startAutoConditioning());
69+
this.adapter.log.info(`Started climate for vehicle ${vin}`);
70+
break;
71+
72+
case 'stop_climate':
73+
await this.writeAndReconcile(`vehicles.${vin}.climate.is_climate_on`, false, () => vehicle.stopAutoConditioning());
74+
this.adapter.log.info(`Stopped climate for vehicle ${vin}`);
75+
break;
76+
77+
case 'start_charging':
78+
await vehicle.startCharging();
79+
this.adapter.log.info(`Started charging for vehicle ${vin}`);
80+
break;
81+
82+
case 'stop_charging':
83+
await vehicle.stopCharging();
84+
this.adapter.log.info(`Stopped charging for vehicle ${vin}`);
85+
break;
86+
87+
case 'flash_lights':
88+
await vehicle.flashLights();
89+
this.adapter.log.info(`Flashed lights for vehicle ${vin}`);
90+
break;
91+
92+
case 'honk_horn':
93+
await vehicle.honkHorn();
94+
this.adapter.log.info(`Honked horn for vehicle ${vin}`);
95+
break;
96+
97+
case 'open_frunk':
98+
await vehicle.actuateTrunk('front');
99+
this.adapter.log.info(`Opened frunk for vehicle ${vin}`);
100+
break;
101+
102+
case 'open_trunk':
103+
await vehicle.actuateTrunk('rear');
104+
this.adapter.log.info(`Opened trunk for vehicle ${vin}`);
105+
break;
106+
107+
default:
108+
this.adapter.log.warn(`Unknown command: ${command}`);
102109
}
103110
}
104111

@@ -112,47 +119,43 @@ export class VehicleHandler {
112119
return;
113120
}
114121

115-
try {
116-
// Handle commands
117-
if (category === 'commands') {
118-
if (value === true || value === 'true') {
119-
await this.executeCommand(vin, stateName);
120-
}
121-
return;
122+
// Handle commands
123+
if (category === 'commands') {
124+
if (value === true || value === 'true') {
125+
await this.executeCommand(vin, stateName);
122126
}
127+
return;
128+
}
123129

124-
// Handle writable states
125-
if (category === 'climate') {
126-
if (stateName === 'driver_temp_setting' || stateName === 'passenger_temp_setting') {
127-
// setTemps takes both temps positionally, so read the other one's
128-
// current value to avoid clobbering it.
129-
const [driverState, passengerState] = await Promise.all([
130-
this.adapter.getStateAsync(`vehicles.${vin}.climate.driver_temp_setting`),
131-
this.adapter.getStateAsync(`vehicles.${vin}.climate.passenger_temp_setting`),
132-
]);
133-
const driverTemp = stateName === 'driver_temp_setting' ? value : (driverState?.val ?? 21);
134-
const passengerTemp = stateName === 'passenger_temp_setting' ? value : (passengerState?.val ?? 21);
135-
// setTemps's positional args are physical left/right seats, not driver/passenger -
136-
// on RHD vehicles the driver sits on the right.
137-
const rhd = this.stateManager.isRhd(vin);
138-
const leftTemp = rhd ? passengerTemp : driverTemp;
139-
const rightTemp = rhd ? driverTemp : passengerTemp;
140-
await vehicle.setTemps(leftTemp, rightTemp);
141-
this.adapter.log.info(`Set temps to ${driverTemp}/${passengerTemp}°C for vehicle ${vin}`);
142-
}
143-
} else if (category === 'charge') {
144-
if (stateName === 'charge_limit_soc') {
145-
await vehicle.setChargeLimit(value);
146-
this.adapter.log.info(`Set charge limit to ${value}% for vehicle ${vin}`);
147-
}
148-
} else if (category === 'state') {
149-
if (stateName === 'sentry_mode') {
150-
await vehicle.setSentryMode(!!value);
151-
this.adapter.log.info(`Set sentry mode to ${value} for vehicle ${vin}`);
152-
}
130+
// Handle writable states
131+
if (category === 'climate') {
132+
if (stateName === 'driver_temp_setting' || stateName === 'passenger_temp_setting') {
133+
// setTemps takes both temps positionally, so read the other one's
134+
// current value to avoid clobbering it.
135+
const [driverState, passengerState] = await Promise.all([
136+
this.adapter.getStateAsync(`vehicles.${vin}.climate.driver_temp_setting`),
137+
this.adapter.getStateAsync(`vehicles.${vin}.climate.passenger_temp_setting`),
138+
]);
139+
const driverTemp = stateName === 'driver_temp_setting' ? value : (driverState?.val ?? 21);
140+
const passengerTemp = stateName === 'passenger_temp_setting' ? value : (passengerState?.val ?? 21);
141+
// setTemps's positional args are physical left/right seats, not driver/passenger -
142+
// on RHD vehicles the driver sits on the right.
143+
const rhd = this.stateManager.isRhd(vin);
144+
const leftTemp = rhd ? passengerTemp : driverTemp;
145+
const rightTemp = rhd ? driverTemp : passengerTemp;
146+
await this.writeAndReconcile(`vehicles.${vin}.climate.${stateName}`, value, () => vehicle.setTemps(leftTemp, rightTemp));
147+
this.adapter.log.info(`Set temps to ${driverTemp}/${passengerTemp}°C for vehicle ${vin}`);
148+
}
149+
} else if (category === 'charge') {
150+
if (stateName === 'charge_limit_soc') {
151+
await this.writeAndReconcile(`vehicles.${vin}.charge.charge_limit_soc`, value, () => vehicle.setChargeLimit(value));
152+
this.adapter.log.info(`Set charge limit to ${value}% for vehicle ${vin}`);
153+
}
154+
} else if (category === 'state') {
155+
if (stateName === 'sentry_mode') {
156+
await this.writeAndReconcile(`vehicles.${vin}.state.sentry_mode`, !!value, () => vehicle.setSentryMode(!!value));
157+
this.adapter.log.info(`Set sentry mode to ${value} for vehicle ${vin}`);
153158
}
154-
} catch (error: any) {
155-
this.adapter.log.error(`Error handling state change for ${vin}.${category}.${stateName}: ${error.message}`);
156159
}
157160
}
158161

0 commit comments

Comments
 (0)