Skip to content

ldw: fix blinker cooldown using wrong time step#37767

Open
Lidang-Jiang wants to merge 4 commits intocommaai:masterfrom
Lidang-Jiang:fix/ldw-blinker-36126
Open

ldw: fix blinker cooldown using wrong time step#37767
Lidang-Jiang wants to merge 4 commits intocommaai:masterfrom
Lidang-Jiang:fix/ldw-blinker-36126

Conversation

@Lidang-Jiang
Copy link
Copy Markdown

Summary

Fix recent_blinker timing bug in LaneDepartureWarning.update(). The blinker cooldown multiplied frame deltas by DT_CTRL (0.01s, 100 Hz control loop rate), but sm.frame advances at DT_MDL (0.05s, 20 Hz model rate). This made the intended 5-second cooldown effectively 25 seconds, causing LDW to remain suppressed far longer than intended after a lane change.

  • Replace DT_CTRL with DT_MDL in import and usage (2 line changes)
  • Add 7 unit tests validating correct timing at frame boundaries

Fixes #36126

Before (3 tests fail — cooldown incorrectly uses DT_CTRL)
============================= test session starts ==============================
platform linux -- Python 3.12.12, pytest-9.0.2, pluggy-1.6.0

selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_dt_mdl_is_used_not_dt_ctrl FAILED [ 14%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_blinker_still_active_within_cooldown PASSED [ 28%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_blinker_cooldown_expires_at_boundary FAILED [ 42%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_blinker_cooldown_math_dt_ctrl_vs_dt_mdl PASSED [ 57%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_right_blinker_cooldown FAILED [ 71%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_no_ldw_when_speed_too_low PASSED [ 85%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_no_ldw_when_lat_active PASSED [100%]

=================================== FAILURES ===================================
___________ TestRecentBlinkerTiming.test_dt_mdl_is_used_not_dt_ctrl ____________

    def test_dt_mdl_is_used_not_dt_ctrl(self):
      """Core regression test: DT_MDL (0.05) must be the multiplier, not DT_CTRL (0.01).

      With DT_MDL=0.05, 5s cooldown = 100 frames.
      With DT_CTRL=0.01, 5s cooldown would be 500 frames.

      At frame 101 after blinker:
        DT_MDL:  101 * 0.05 = 5.05s >= 5.0 => cooldown expired => LDW triggers
        DT_CTRL: 101 * 0.01 = 1.01s <  5.0 => cooldown active  => LDW suppressed (BUG)
      """
      ...
      ldw.update(101, model, cs_no_blinker, cc)
>     assert ldw.left is True, (
        "LDW should trigger after 101 model frames (~5.05s with DT_MDL); "
        "if this fails, DT_CTRL is likely still in use"
      )
E     AssertionError: LDW should trigger after 101 model frames (~5.05s with DT_MDL);
      if this fails, DT_CTRL is likely still in use

=================== 3 failed, 4 passed in 0.09s ====================
After (all 7 tests pass)
============================= test session starts ==============================
platform linux -- Python 3.12.12, pytest-9.0.2, pluggy-1.6.0

selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_dt_mdl_is_used_not_dt_ctrl PASSED [ 14%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_blinker_still_active_within_cooldown PASSED [ 28%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_blinker_cooldown_expires_at_boundary PASSED [ 42%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_blinker_cooldown_math_dt_ctrl_vs_dt_mdl PASSED [ 57%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_right_blinker_cooldown PASSED [ 71%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_no_ldw_when_speed_too_low PASSED [ 85%]
selfdrive/controls/tests/test_ldw.py::TestRecentBlinkerTiming::test_no_ldw_when_lat_active PASSED [100%]

======================== 7 passed in 0.04s =========================

Test plan

  • 7 unit tests with mock stubs (no full openpilot build required)
  • Core regression test: verifies LDW triggers at 101 model frames (5.05s) — would fail with DT_CTRL
  • Boundary test: verifies cooldown expires at exactly frame 100 (5.0s)
  • Numerical demonstration: DT_CTRL produces 1.01s vs DT_MDL produces 5.05s at 101 frames

recent_blinker multiplied frame deltas by DT_CTRL (100 Hz) but
sm.frame advances at DT_MDL (20 Hz), making the 5-second cooldown
effectively 25 seconds. Replace DT_CTRL with DT_MDL.

Fixes commaai#36126

Signed-off-by: Lidang-Jiang <[email protected]>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 5, 2026

Process replay diff report

Replays driving segments through this PR and compares the behavior to master.
Please review any changes carefully to ensure they are expected.

✅ 0 changed, 66 passed, 0 errors

- Replace unittest.mock.MagicMock with types.SimpleNamespace (TID251: unittest banned)
- Remove unused imports: pytest, importlib (F401)
- Use openpilot.selfdrive prefix instead of bare selfdrive (TID251)
- Use explicit string concatenation instead of implicit (ISC002)
- Remove unused noqa directives (RUF100)

Signed-off-by: Lidang-Jiang <[email protected]>
The previous approach stubbed cereal and openpilot namespace modules at
import time, which polluted sys.modules for the entire pytest session
and caused ImportError in unrelated tests (test_leads, test_longcontrol,
etc.). Since CI builds cereal properly, use direct imports instead.

Signed-off-by: Lidang-Jiang <[email protected]>
laneChangeLeft=3, laneChangeRight=4, so the mock list needs 7 elements
instead of 3. This fixes the IndexError in CI unit tests.

Signed-off-by: Lidang-Jiang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plannerd: recent_blinker bug in LDW.

1 participant