vwegolf: set the pre-conditioning target temperature (BC_SET_TEMP) - #1507
Open
Nueueuet wants to merge 1 commit into
Open
vwegolf: set the pre-conditioning target temperature (BC_SET_TEMP)#1507Nueueuet wants to merge 1 commit into
Nueueuet wants to merge 1 commit into
Conversation
Adds the one BatteryControl setting the module could not change yet. Same shape as BC_SET_CURRENT: read profile 0, change one field, write it back, confirm on the write echo, no trigger. Only the temperature byte moves; the operation byte is left exactly as the car has it, so this can neither start nor stop anything. The raw profile encoding (degC * 10 - 100) is passed as the command param so it fits the existing uint8_t: 15.5 C is 55, 30.0 C is 200. Three places needed the new op alongside BC_SET_CURRENT, so they share an IsSettingsEdit() predicate now: the post-arm phase choice, the lost-echo retry, and the skip-write optimisation. That last one mattered most - it compares only the operation byte, so a temperature-only edit looked like a no-op and nothing was written at all, after which the trigger fired. Confirmation also lives inside the BC_SET_CURRENT branch of the write-echo handler; without its own the command never completed and re-armed every two seconds until the window closed. Also adds two commands the Android app uses, since the target temperature is not carried by the v2 protocol and cannot be read app-side from metrics: xvg cctemp <15.5..30.0> set it, snapped to the half-degree steps xvg ccstatus cctemp=22.0 current=32 valid=1 valid checks the temperature against 0: below 10 C is not encodable, so 0 can only mean not read yet. Unlike the charge current, a running conditioning session picks the new setpoint up by itself, so there is no re-fire. Measured on a 2016 e-Golf at 28 C ambient: at a 30 C target with a 35 C cabin it drifted at -0.19 C/min, and about 40 s after writing 16.0 C the rate went to -2.2 C/min; the other way round it turned from -1.3 C/min to +0.29 C/min within the same ~40 s. Documented in bat-ctrl-bap.md next to the charge-current note it contrasts with. Two small additions were needed to keep the native tests building: the mock's OvmsWriter gained puts/printf and OvmsMetric an AsInt(), both of which the new ccstatus command uses, and vehicle_vwegolf.cpp now includes <cmath> for lroundf instead of picking it up transitively as it does on the target. 136/136 pass. 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
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.
Follow-up to #1496, as suggested by @SCjona: the one BatteryControl setting his
module could not change yet.
BC_SET_TEMPis shaped like hisBC_SET_CURRENT— read profile 0, change onefield, write it back, confirm on the write echo, no trigger. Only the
temperature byte moves; the operation byte is left exactly as the car has it, so
it can neither start nor stop anything. The raw profile encoding
(
degC * 10 - 100) goes in as the command param so it fits the existinguint8_t: 15.5 °C is 55, 30.0 °C is 200.Three places needed the new op alongside
BC_SET_CURRENT, so they share anIsSettingsEdit()predicate now: the post-arm phase choice, the lost-echo retry,and the skip-write optimisation. That last one is the one that bites — it
compares only the operation byte, so a temperature-only edit looks like a no-op,
nothing is written at all, and the trigger fires anyway. Confirmation also lives
inside the
BC_SET_CURRENTbranch of the write-echo handler; without its own thecommand never completes and re-arms every two seconds until the window closes.
Both are worth a suspicious look from whoever knows that state machine.
Two commands come with it, since the target temperature is not carried by the v2
protocol and cannot be read app-side from metrics:
validtests the temperature against 0, per @SCjona's suggestion: below 10 °C isnot encodable, so 0 can only mean "not read yet".
Does the BCU apply it mid-run?
@SCjona expected it to behave like the charge current, which a running charge
ignores until the next start. It does not — the setpoint is picked up while
conditioning runs, so there is no re-fire. Measured on a 2016 e-Golf, plugged in,
28 °C ambient,
v.e.cabintemppolled every 20 s:Three and a half minutes at a 30 °C target with a 35 °C cabin and it barely
moved. About 40 s after the write the curve bends and the cooling rate goes up
tenfold. The reverse, in a second run:
Cooling to heating, no restart, same ~40 s latency. Both runs are in the log with
the
49 59 bxwrite echoes;v.e.hvacstayed set throughout and the only49 58 00 -> HVAC OFFin either came from the stop command. This contrast isdocumented in
bat-ctrl-bap.mdnext to the charge-current note.Notes
OvmsWritergained
puts/printfandOvmsMetricanAsInt(), both used byccstatus,and
vehicle_vwegolf.cppnow includes<cmath>forlroundfinstead ofpicking it up transitively as it does on the target. 136/136 pass.
TransmitMsgCapabilities(). That is a separate concern and is now Server V2: advertise capability C26 so the app can offer climate control #1506.