Skip to content

selfdrived: add permanent alert for processNotRunning#38238

Open
srijavuppala wants to merge 1 commit into
commaai:masterfrom
srijavuppala:fix-process-not-running-alert-shadowed
Open

selfdrived: add permanent alert for processNotRunning#38238
srijavuppala wants to merge 1 commit into
commaai:masterfrom
srijavuppala:fix-process-not-running-alert-shadowed

Conversation

@srijavuppala

Copy link
Copy Markdown

Summary

Fixes #34751.

processNotRunning only defines ET.NO_ENTRY and ET.SOFT_DISABLE alerts. The SOFT_DISABLE alert is transient (2-4s depending on soft_disable_time), and NO_ENTRY is only ever included in current_alert_types while the driver is actively trying to engage (see StateMachine.update in selfdrive/selfdrived/state.py, ET.PERMANENT is always active but ET.NO_ENTRY is only appended inside the events.contains(ET.ENABLE) branch).

So once a process dies, after both of those alerts expire there is nothing left in EVENTS[processNotRunning] for ET.PERMANENT, the alert type that's always considered. If a consequential fault is also active and has a PERMANENT alert (e.g. steerUnavailable's "LKAS Fault: Restart the car to engage"), that unrelated, lower-severity alert is the only thing left to display, masking the real cause exactly as described in the issue.

modeldLagging already follows the pattern this needs: SOFT_DISABLE + NO_ENTRY + PERMANENT. This PR adds the same PERMANENT alert for processNotRunning, at Priority.LOW (one step above the default Priority.LOWER used by NormalPermanentAlert/steerUnavailable's permanent alert) so it reliably outranks other default-priority permanent alerts that are just side effects of the same underlying process crash, regardless of which one happened to start first.

Test plan

  • ruff check passes
  • Traced AlertManager.process_alerts (selfdrive/selfdrived/alertmanager.py) to confirm selection is max(priority, start_frame) across all active alerts, and reproduced both the before/after outcome with a standalone script mirroring that exact comparison using the real Priority values:
    • before: processNotRunning contributes no PERMANENT alert, so steerUnavailable's PERMANENT (Priority.LOWER) wins by default
    • after: processNotRunning's new PERMANENT alert (Priority.LOW) wins regardless of start order
  • Confirmed against the route from the issue (ea1f52a90cc05090/00000001--1fc11d46d7) that processNotRunning was present in onroadEvents with no PERMANENT entry, while steerUnavailable was permanent and is what actually displayed

processNotRunning only had NO_ENTRY and SOFT_DISABLE alerts. The
SOFT_DISABLE alert is transient (2-4s), and NO_ENTRY only shows while
the driver is actively trying to engage. Once both expire, there is
nothing left to display for it, so an unrelated lower-priority
permanent alert (e.g. steerUnavailable's 'LKAS Fault: Restart the car
to engage') ends up shown instead, masking the real cause of the
disengagement.

Add a PERMANENT alert for processNotRunning at Priority.LOW, following
the same pattern already used for modeldLagging, so it keeps showing
and outranks other default-priority (LOWER) permanent alerts.
@github-actions

Copy link
Copy Markdown
Contributor

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

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.

process not running alert can be shadowed

1 participant