Skip to content

Commit 1c30d47

Browse files
authored
fix(funnel): treat any unlocked vehicle lock state as unlocked (#134)
* feat(funnel): widen lock-state mapping to treat any unlocked state as unlocked Map VEHICLELOCKSTATE_INTERNAL_LOCKED as locked and VEHICLELOCKSTATE_SELECTIVE_UNLOCKED as unlocked in _LOCK_STATES, and update the comment to state the current rule instead of the prior unvalidated-reasoning note. UNKNOWN and FAILED_UNLATCH remain unmapped. * no-mistakes(review): docs: fix stale CLAUDE.md sentence on lock-state mapping
1 parent 8bccb5a commit 1c30d47

3 files changed

Lines changed: 13 additions & 8 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Commands (vehicle/commands.py) - protobuf-based signed command implementation (A
6969

7070
`Router` (`router/base.py`) is an entity-agnostic composition wrapper (not part of the inheritance chain) that chains an ordered list of two-or-more backends sharing a common method surface and dispatches each method call down the chain with automatic per-command failover: it tries the first backend that has the method and, on any exception except `BluetoothUnconfirmedCommand`, retries the same call on the next backend that has it, returning the first success (raising the last error only if every applicable backend fails, `AttributeError` only if none has the method). Non-callable attributes resolve to the first backend that has them. Constructor: `Router(primary, secondary, *more_backends, health=None)`. The health check (`bool` | sync callable | async callable returning `bool`; omitted = attempt primary, fail over on exception with no probe) gates **only the primary**; the rest of the chain is reached purely through per-command failover — there is deliberately no per-backend health matrix. Double-execution caveat: a non-idempotent command that fails mid-flight can be re-run on the next backend, except for `BluetoothUnconfirmedCommand`, which propagates without replay.
7171

72-
`ObservationFunnel` (`funnel.py`) is the **read** side, a separate mechanism from the command `Router` and not in its inheritance chain. It is a **funnel, not a selector**: every attached publisher feeds the same per-field listeners, so a field bound to one source survives that source dropping. There is deliberately no source health, availability, grace window, failback delay, priority, stickiness or per-field selection anywhere in it — **unavailability is a value a source reports** (a null/SNA reading), never something the funnel infers from a link dropping; inferring it would be the funnel asserting data it does not have. The only arbitration is `publish()` ignoring an observation older than the last one for that field and not re-dispatching an unchanged value; both are hard-coded, not configurable. It is **entirely synchronous and can never originate a request**: no `async def`/`await`, no polling loop, no request callable, no scheduling task — `tests/test_funnel.py::TestFunnelCannotOriginateWork` locks that in against the module's own AST, so keep the module synchronous rather than adding a fetch path. Polling belongs entirely to an external consumer, which may gate its own schedule on `listen_demand(paths, cb)` (a read-only observer over the activation counts) and feed results back through `VehicleDataResultPublisher.publish_result(dict)` — that publisher holds no client, session, or callable able to obtain one. Publishers push into the funnel (which is itself the `ObservationSink`) via `publish(Observation)`; `observed_at` values must come from one monotonic clock shared by every publisher on a funnel. `value(path)` returns the last observed value, its `None` meaning either never observed or reported unavailable. Fields are deliberately three (`Locked`, `ChargePortDoorOpen`, `DoorState.TrunkFront`); translations are positive allowlists, and an unmapped VCSEC enum or absent JSON leaf emits no observation rather than a guess, while an explicit JSON null emits an unavailable value. `BleBroadcastPublisher` reuses the existing `VehicleBluetooth` `listen_vehicle_lock_state`/`listen_charge_port`/`listen_front_trunk` seams and never connects, reads, or commands; because `VEHICLELOCKSTATE_UNLOCKED` is 0 with no proto3 presence, every VCSEC status broadcast reports a lock state and the funnel deduplicates the repeats. VCSEC `INTERNAL_LOCKED`/`SELECTIVE_UNLOCKED` and closure `UNKNOWN`/`FAILED_UNLATCH` are unmapped pending live-frame validation.
72+
`ObservationFunnel` (`funnel.py`) is the **read** side, a separate mechanism from the command `Router` and not in its inheritance chain. It is a **funnel, not a selector**: every attached publisher feeds the same per-field listeners, so a field bound to one source survives that source dropping. There is deliberately no source health, availability, grace window, failback delay, priority, stickiness or per-field selection anywhere in it — **unavailability is a value a source reports** (a null/SNA reading), never something the funnel infers from a link dropping; inferring it would be the funnel asserting data it does not have. The only arbitration is `publish()` ignoring an observation older than the last one for that field and not re-dispatching an unchanged value; both are hard-coded, not configurable. It is **entirely synchronous and can never originate a request**: no `async def`/`await`, no polling loop, no request callable, no scheduling task — `tests/test_funnel.py::TestFunnelCannotOriginateWork` locks that in against the module's own AST, so keep the module synchronous rather than adding a fetch path. Polling belongs entirely to an external consumer, which may gate its own schedule on `listen_demand(paths, cb)` (a read-only observer over the activation counts) and feed results back through `VehicleDataResultPublisher.publish_result(dict)` — that publisher holds no client, session, or callable able to obtain one. Publishers push into the funnel (which is itself the `ObservationSink`) via `publish(Observation)`; `observed_at` values must come from one monotonic clock shared by every publisher on a funnel. `value(path)` returns the last observed value, its `None` meaning either never observed or reported unavailable. Fields are deliberately three (`Locked`, `ChargePortDoorOpen`, `DoorState.TrunkFront`); translations are positive allowlists, and an unmapped VCSEC enum or absent JSON leaf emits no observation rather than a guess, while an explicit JSON null emits an unavailable value. `BleBroadcastPublisher` reuses the existing `VehicleBluetooth` `listen_vehicle_lock_state`/`listen_charge_port`/`listen_front_trunk` seams and never connects, reads, or commands; because `VEHICLELOCKSTATE_UNLOCKED` is 0 with no proto3 presence, every VCSEC status broadcast reports a lock state and the funnel deduplicates the repeats. Any unlocked VCSEC lock state, including `INTERNAL_LOCKED`→locked and `SELECTIVE_UNLOCKED`→unlocked, maps to a boolean per the "any unlocked is unlocked" ruling; closure `UNKNOWN`/`FAILED_UNLATCH` remain unmapped pending live-frame validation.
7373

7474
`VehicleRouter` and `EnergySiteRouter` (`router/vehicle.py`, `router/energysite.py`) are thin entity-specific `Router` subclasses. `VehicleRouter(bluetooth_primary, teslemetry_secondary)` pairs a `VehicleBluetooth` primary with a cloud (`TeslemetryVehicle`) secondary; `EnergySiteRouter(local_energysite, teslemetry_energysite)` pairs a duck-typed local `EnergySite`-shaped object (e.g. aiopowerwall's `PowerwallEnergySite`, no dependency added) with a cloud `TeslemetryEnergySite` fallback. Both re-export from `router/__init__.py` (`tesla_fleet_api.router.Router` etc.) and from `tesla/__init__.py` (`tesla_fleet_api.tesla.Router`) for backward compatibility. They have no factory on the `Vehicles`/`EnergySites` collections. This repo owns the RSA keypair lifecycle and cloud registration (`Tesla.get_rsa_private_key`, `EnergySite.add_authorized_client`) that aiopowerwall's local signed transport depends on but does not implement itself; see `docs/energy_local_control.md` for the end-to-end pairing + `EnergySiteRouter` composition flow. The cloud-only `set_island_mode`/`go_off_grid`/`reconnect_grid` (`tesla/energysite.py`) can only send an unsigned `grpc_command`, which gateways can acknowledge without actuating the contactor — rather than ship that as a silent no-op, they unconditionally raise `SignedCommandRequired` (`exceptions.py`); only the signed local path via `add_authorized_client` + `EnergySiteRouter` actually actuates, and a success response from that transport still doesn't prove the contactor moved — verify state after the call.
7575

tesla_fleet_api/funnel.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -266,15 +266,18 @@ def _notify_demand(self) -> None:
266266
# UNLOCKED is 0 and the enum has no proto3 presence, so every status broadcast
267267
# reports a lock state and the funnel deduplicates the repeats.
268268
#
269-
# INTERNAL_LOCKED and SELECTIVE_UNLOCKED are deliberately unmapped: reducing
270-
# either to one boolean is unvalidated against live frames, and emitting
271-
# nothing keeps the last confirmed value instead of guessing.
269+
# Only a locked state reads as locked; any unlocked state, however partial
270+
# or selective, reads as unlocked.
272271
_LOCK_STATES: Mapping[int, bool] = {
273272
VehicleLockState_E.VEHICLELOCKSTATE_LOCKED: True,
274273
VehicleLockState_E.VEHICLELOCKSTATE_UNLOCKED: False,
274+
VehicleLockState_E.VEHICLELOCKSTATE_INTERNAL_LOCKED: True,
275+
VehicleLockState_E.VEHICLELOCKSTATE_SELECTIVE_UNLOCKED: False,
275276
}
276277

277-
# UNKNOWN and FAILED_UNLATCH are unmapped for the same reason.
278+
# UNKNOWN and FAILED_UNLATCH are unmapped: reducing either to one boolean is
279+
# unvalidated against live frames, and emitting nothing keeps the last
280+
# confirmed value instead of guessing.
278281
_CLOSURE_STATES: Mapping[int, bool] = {
279282
ClosureState_E.CLOSURESTATE_CLOSED: False,
280283
ClosureState_E.CLOSURESTATE_OPEN: True,

tests/test_funnel_bluetooth.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,7 @@ def test_ordinary_lock_states_map_to_booleans(self) -> None:
116116
[(FieldPath.LOCKED, True), (FieldPath.LOCKED, False)],
117117
)
118118

119-
def test_unvalidated_lock_states_emit_no_observation(self) -> None:
120-
"""An unmapped enum keeps the last confirmed value instead of guessing."""
119+
def test_partial_lock_states_map_to_booleans(self) -> None:
121120
vehicle = _make_vehicle()
122121
_, sink = _attached(vehicle)
123122

@@ -126,7 +125,10 @@ def test_unvalidated_lock_states_emit_no_observation(self) -> None:
126125
_lock(VehicleLockState_E.VEHICLELOCKSTATE_SELECTIVE_UNLOCKED)
127126
)
128127

129-
self.assertEqual(sink.observations, [])
128+
self.assertEqual(
129+
[(o.path, o.value) for o in sink.observations],
130+
[(FieldPath.LOCKED, True), (FieldPath.LOCKED, False)],
131+
)
130132

131133
def test_closure_states_map_to_booleans(self) -> None:
132134
vehicle = _make_vehicle()

0 commit comments

Comments
 (0)