Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: DM icon shows engaged IF always-on #33183

Closed
wants to merge 6 commits into from

Conversation

ugtthis
Copy link
Contributor

@ugtthis ugtthis commented Aug 5, 2024

Problem: DM icon only shows engaged when openpilot is engaged. DM icon does not show it is engaged if "Always-on driver monitoring" is enabled.

Solution: Make DM Icon show as engaged(with green tracking arcs as shown below) when either openpilot is engaged OR when Always-on driver monitoring is enabled.

dm-engaged


DM icon not engaged showing grey tracking arc color

dm-icon-not-engaged

@github-actions github-actions bot added the ui label Aug 5, 2024
Copy link
Contributor

github-actions bot commented Aug 5, 2024

Thanks for contributing to openpilot! In order for us to review your PR as quickly as possible, check the following:

  • Convert your PR to a draft unless it's ready to review
  • Read the contributing docs
  • Before marking as "ready for review", ensure:
    • the goal is clearly stated in the description
    • all the tests are passing
    • the change is something we merge
    • include a route or your device' dongle ID if relevant

Copy link
Contributor

github-actions bot commented Aug 5, 2024

UI Screenshots

@deanlee
Copy link
Contributor

deanlee commented Aug 5, 2024

Added some suggestions.

@ugtthis
Copy link
Contributor Author

ugtthis commented Aug 5, 2024

Can you repost them? For some reason I don't see any suggestions from you on my end

selfdrive/ui/qt/onroad/annotated_camera.cc Outdated Show resolved Hide resolved
selfdrive/ui/qt/onroad/annotated_camera.cc Outdated Show resolved Hide resolved
@ugtthis ugtthis marked this pull request as draft August 5, 2024 13:24
@FrogAi
Copy link
Contributor

FrogAi commented Aug 5, 2024

@ugtthis I made a PR to your repo that cleans up the usage of the params.

@ugtthis
Copy link
Contributor Author

ugtthis commented Aug 5, 2024

@FrogAi and @deanlee thanks guys for cleaning up my code. Learned a lot thanks!

@sshane sshane requested a review from ZwX1616 August 5, 2024 19:22
@sshane
Copy link
Contributor

sshane commented Aug 5, 2024

@ZwX1616 Do we want to keep this engaged-only for any ux reason?

@ugtthis ugtthis marked this pull request as ready for review August 5, 2024 19:24
@ZwX1616
Copy link
Contributor

ZwX1616 commented Aug 9, 2024

@ZwX1616 Do we want to keep this engaged-only for any ux reason?

Yes green should be consistent with engaged state

@ZwX1616 ZwX1616 closed this Aug 9, 2024
@ugtthis
Copy link
Contributor Author

ugtthis commented Aug 9, 2024

It still is green when engaged for this PR. But it also gets it to turn green when people toggle the always-on driver monitoring to show that DM is engaged even when openpilot is not engaged. This is to show users clearer when DM is on, and when it is not.

This PR came about when I was driving without openpilot engaged and I was getting DM alerts saying to pay attention and I was confused why, then went to settings and saw I had "Always-On Driver Monitoring" enabled

@ugtthis
Copy link
Contributor Author

ugtthis commented Aug 9, 2024

I don’t think users go into settings every time they use openpilot. And currently that would be the only way to check if your “Always-On Driver monitoring” is engaged for the trip.

For people who are using this always-on DM feature when openpilot is not engaged and don’t check their settings every drive, how would they know whether it’s not alerting them to pay attention/wake up because DM is off or whether the DM just needs to get better?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants