Honor speed limit priority for upcoming limits - #102
Closed
FrogAi wants to merge 2 commits into
Closed
Conversation
FrogAi
force-pushed
the
codex/honor-upcoming-speed-limit-priority
branch
from
August 8, 2026 23:35
3528ae0 to
110a826
Compare
FrogAi
force-pushed
the
codex/honor-upcoming-speed-limit-priority
branch
from
August 9, 2026 16:43
110a826 to
a0454e4
Compare
Contributor
Author
|
Closing this because its premise conflicts with the intended behavior: upcoming limits are map-derived, and the upcoming-limit feature deliberately ignores current-limit source priority. Applying PrioritySpeedLimit here can replace or suppress valid map lookahead under some priority settings. The branch and evidence are preserved, but this proposal should not merge. |
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.
Summary
PrioritySpeedLimitto the upcoming map candidate before lookahead uses it.TriggerDistanceactivation hysteresis when the arbitrated upcoming target changes, so a transient external value or a mid-approach priority change cannot latch a stale activation distance.nextSpeedLimittelemetry as the raw upcoming map value.Motivation
The current limit already passes through configured map/external source arbitration, but the upcoming limit did not. Lookahead used raw
NextLimit.Valuefrom the map for every control calculation, so external-only, external-priority, highest, and lowest modes could switch back to the map source as soon as an upcoming limit entered range.For example, with external-only control selecting
20 m/sand an upcoming map limit of10 m/s, the baseline path substitutes10 m/s. When acceptance and enable-speed gating permit it, that value can become the final controller cap.The fix stays at the control-consumer boundary. Upcoming map discovery and the raw
nextSpeedLimitoutput remain unchanged.Behavior
68813e05110a826Values are m/s with zero offset. The expected targets come from a static code-path trace with fresh trigger state, the upcoming limit inside activation distance, and the direction-appropriate slowdown or speed-up option enabled.
Offset is applied after arbitration. With slowdown enabled and in range, an external-only
30 m/starget, upcoming map value10 m/s, and3 m/soffset changes from the baseline13 m/sto33 m/s.Because the activation distance is now derived from the arbitrated value — which can change between ticks via external updates or priority/enable settings — the monotonic
TriggerDistanceratchet (previously reset only when the upcoming way changed) could latch a distance computed from a transient arbitration result. On10e8d70, one transient external frame of100 m/sinhighestmode latched a ~4000 m trigger and kept substituting the upcoming13.9 m/slimit from 2 km out after the external value recovered; base68813e05is immune because its activation distance only used the per-approach-constant raw map value.110a826zeroes the ratchet whenever the arbitrated target changes, scoping the hysteresis to the target it was computed from. Within a constant arbitration regime (map-only, or map priority with a map value present) the ratchet behavior is identical to base.Validation
30 m/s, selected external20 m/s, and upcoming map10 m/s: the observed target was10 m/sinstead of20 m/s.30 m/s, external20 m/s, upcoming map10 m/s, offset2 m/s, and fresh trigger state. The corrected jerk and time-offset calculations use the priority-selected22 m/starget; the baseline uses the raw-map12 m/starget.68813e05as expected and the 7 behavior-preservation cases pass on both.10e8d70(one external frame of100 m/sinhighestmode, then cleared to0: the suggestion dropped to13.9 m/swhile still 2000 m out, with the ratchet latched at ~4033 m) and confirmed base68813e05immune and110a826corrected (the suggestion stays27.8 m/s).10e8d70; Fork Build #19 passed themake buildjob on exact head commit110a826.Compatibility
Rebased onto current
main(a0454e4)mainhas since restructured settings into sub-structs and split the lookahead time offset by direction, so this PR was rebased rather than merged mechanically. The resolution keepsmain'sSpeedLimitSettingspaths and its personality-basedSpeedLimitIncreaseTargetSpeedTimeOffset/SpeedLimitDecreaseTargetSpeedTimeOffsetbranches, and applies this PR's arbitration on top: the upcoming limit is resolved throughPrioritySpeedLimitfirst, a zero result returns the current suggestion, the trigger-distance ratchet resets when the arbitrated target changes, and both direction branches now scaleoffsetNextSpeedLimitinstead of recomputing the raw value.Re-verified against the rebased base: the full 14-case priority matrix (map-only, external-only, external-priority with and without a value, highest, lowest, map-priority, hold-last-seen, disabled controls, and activation gating) passes, as does the multi-tick trigger-distance regression — a transient external spike no longer latches an oversized activation distance. FrogAi Build #57 passed for the exact head.
One test-harness note worth recording for anyone re-running this: the zero value of
LongitudinalPersonalityselects Aggressive, not Standard, so a fixture that populates onlyPersonalities.Standardsilently exercises an all-zero personality and every lookahead toggle reads false.