fix(obd): keep one ELM327 prompt to one response - #64
Merged
Conversation
The RX buffer was drained with a single check and then cleared outright:
if (responseBuffer.contains(">")) {
val fullResponse = responseBuffer.toString().trim()
responseBuffer.clear()
pendingDeferreds[device.id]?.complete(fullResponse)
}
A BLE notification can carry more than one prompt. When it does, both replies
were handed over as one string and everything after the first prompt was
thrown away, so the next command received the previous command's text and
every reply after it was off by one. A partial reply left over from a command
that had already timed out was prepended to the next one the same way.
Drain the buffer the way the ESPHome ble_elm327 component does: cut at each
prompt in a loop, keep what follows for the next notification, and hand each
reply to one waiting command. A reply with no command waiting is a late
arrival and is dropped. The buffer is also reset before each write and on
teardown so a stale fragment cannot leak into a fresh command.
Also stop choosing the read timeout by command length. `cmd.length >= 6` was
standing in for "this might be multi-frame", which mode 21 block requests are
too — 21 03 answers with more than 60 bytes but is only four characters long.
Give AT commands the short timeout and every data request the long one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The chunks concatenate directly, so "0:6103AA" followed by "BBCC\r1:DDEE\r>" gives "0:6103AABBCC\r1:DDEE" — the assertion expected a carriage return between AA and BBCC that was never in the input. Split the response at a clearer boundary instead, so the case reads as one reply arriving in two notifications. The production logic was correct; only the assertion was wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The RX buffer was drained with a single check and then cleared outright:
A BLE notification can carry more than one prompt. When it does, both replies were handed over as one string and everything after the first prompt was thrown away, so the next command received the previous command's text and every reply after it was off by one. A partial reply left over from a command that had already timed out was prepended to the next one the same way.
Drain the buffer the way the ESPHome ble_elm327 component does: cut at each prompt in a loop, keep what follows for the next notification, and hand each reply to one waiting command. A reply with no command waiting is a late arrival and is dropped. The buffer is also reset before each write and on teardown so a stale fragment cannot leak into a fresh command.
Also stop choosing the read timeout by command length.
cmd.length >= 6was standing in for "this might be multi-frame", which mode 21 block requests are too — 21 03 answers with more than 60 bytes but is only four characters long. Give AT commands the short timeout and every data request the long one.