obd: stop posting "adapter asleep or car off" on every boot (#56, #31) - #65
ThinkOffApp wants to merge 1 commit into
Conversation
@eclass sent petrus the identical line at 12:58Z and 13:06Z on 15 Sep with the car parked at home. Two faults, one root cause. `no_data_posted` was a local variable. obdwatch restarts on every boot and every rfcomm rebind, so each fresh process started with the flag cleared and re-sent the same line to his phone. This file already learned that lesson once - DEEP_STAMP a few lines above is persisted precisely because the in-memory `deep_done` reset on rfcomm flaps - and it was not applied here. The epoch marker now lives on disk (#31). The second fault is that the line should not be sent on a cold boot at all. Waking up next to a parked car is not an event, it is the normal resting state, and it was arriving as a notification. It is now gated on `seen_data`, so it posts only as a genuine data -> lost TRANSITION, which is what #56 asks for. The resting fact still shows in the presence and preflight tiles, where a state belongs. A suppressed post is logged, so the behaviour is visible in the journal rather than silent. Stamp bookkeeping never raises: a read-only or missing state dir logs and carries on, because losing the loop is worse than one duplicate post. Tests cover persistence across a restart, clearing on a successful reading, clearing an absent marker, an unwritable stamp path, and an assertion that the `seen_data` gate is still in the loop so a refactor cannot quietly drop it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea8f2a91d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # made the "LIVE 29s" badge grow to a minute between updates). | ||
| else: | ||
| if not last_post_readings and not no_data_posted: | ||
| if not last_post_readings and not no_data_posted and seen_data: |
There was a problem hiding this comment.
When a successful poll is followed by result["ok"] == False while the adapter path remains present, last_post_readings still contains the successful line, so this condition is false regardless of seen_data. Because the persistent rfcomm service can leave /dev/rfcomm0 bound while the gateway stops answering, the intended data-to-lost transition can occur without any reconnect clearing this value, and the new alert is never posted.
Useful? React with 👍 / 👎.
| elif not last_post_readings and not seen_data: | ||
| print("obdwatch: no engine data and none seen this session " | ||
| "- not posting (car is simply off)", flush=True) | ||
| last_post_readings = "(failed)" |
There was a problem hiding this comment.
On the targeted cold-boot path, an asleep car reaches this branch and stores the truthy "(failed)" sentinel. If the ignition subsequently wakes the gateway without /dev/rfcomm0 disappearing, the first successful poll has first == False, no previous DTC with which to detect a change, and no battery baseline for a milestone; therefore it is not posted, seen_data remains false, and the stamp is not cleared. Ordinary later successes remain in the same state, suppressing both the expected initial engine reading and future loss notifications for the lifetime of the process.
Useful? React with 👍 / 👎.
2 participants
Fixes #56. Also fixes #31, which is the same root cause.
@eclass sent petrus the identical line at 12:58Z and 13:06Z on 15 Sep, car parked at home.
Two faults, one cause
1. The epoch marker was a local variable.
no_data_posted = False— obdwatch restarts on every boot and every rfcomm rebind, so each fresh process cleared the flag and re-sent the line.This file already learned that lesson. A few lines above:
Same failure, same file, not applied. The marker now lives on disk — that is #31.
2. It should not be sent on a cold boot at all. Waking up next to a parked car is not an event, it is the normal resting state, and it was arriving as a phone notification. Now gated on
seen_data, so it fires only as a real data → lost transition, which is what #56 asks for. The resting fact still appears in the presence and preflight tiles, where a state belongs rather than an alert.A suppressed post is logged, so the behaviour is visible in the journal instead of silent.
Safety
Stamp bookkeeping never raises — a read-only or missing state dir logs and carries on. Losing the read loop is worse than one duplicate post.
Tests
tests/test_no_data_post.py, 5 cases: persistence across a restart, clearing on a successful reading, clearing an absent marker, an unwritable stamp path, and an assertion that theseen_datagate is still present in the loop so a refactor cannot quietly drop it.The suite's one failure on this branch is
test_dash_loop's missing pytest, which is pre-existing onmainand fixed separately in #64.Not verified on the car
The car is not reachable right now (no vadelma node in tailscale). This is reasoned from the code path and the timestamps in the issue, and the tests exercise the marker directly — but the boot behaviour itself has not been observed on Vadelma.
🤖 Generated with Claude Code