controls: push Resume blocking down to car port

[jyoung8607]

Description

Move the resume block from #25402 down into the GM car port, so that other car ports can use Resume for first engagement.

Verification

In progress.

Route

Route: TBD

[sshane]

I remember the reason being that this was a feature we wanted to enforce consistency across the cars + it would be too much code to support in GM's interface, but this seems simpler than I thought.

I didn't see any thing about the set speed state machine behavior in ISO 15622:2018, only for ACC. Any preference on this @adeebshihadeh?

In all our cars here, I don't think I've driven one that let's you press the resume button before setting a speed (besides HKG main button), that's probably good to match, unless there's cars that don't do this.

[andiradulescu]

Sorry for spamming this PR, I didn’t realise I was doing it.

I would really love to see this merged!

@adeebshihadeh what do you think?

[jyoung8607]

In earlier conversations with the openpilot team, I believe they were open to discussing it. It's in Draft right now because there's a process replay diff with GM, possibly due to different rising/falling edge behavior. If there's going to be a process replay diff, it needs to be fully explained (which I can't yet) and it needs to be tested on a GM car (which I haven't asked anyone to do yet).

TL;DR: I can't yet prove this doesn't break GM in its current form.

[adeebshihadeh]

I remember the reason being that this was a feature we wanted to enforce consistency across the cars + it would be too much code to support in GM's interface, but this seems simpler than I thought.

I didn't see any thing about the set speed state machine behavior in ISO 15622:2018, only for ACC. Any preference on this @adeebshihadeh?

In all our cars here, I don't think I've driven one that let's you press the resume button before setting a speed (besides HKG main button), that's probably good to match, unless there's cars that don't do this.

Don't have a strong preference here, aside from generally matching the stock behavior to prevent confusion.

[sshane]

@jyoung8607 are you aware of any stock ACC system that allows you to press the resume/+ button on first engage?

[jyoung8607]

@jyoung8607 are you aware of any stock ACC system that allows you to press the resume/+ button on first engage?

Volkswagen. It's the way I (and many others) use openpilot with stock ACC. The current behavior is a hard POLA violation.

Can't speak for other cars. If you'd like this PR to support it on a whitelist basis only, that would be reasonable.

[github-actions]

This PR has had no activity for 30 days. It will be automatically closed in 7 days if there is no activity.

[github-actions]

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

[github-actions]

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

[github-actions]

This PR has had no activity for 14 days. It will be automatically closed in 3 days if there is no activity.

[sshane]

We've moved the car interfacing code to our opendbc repository, which is now the new home for car ports and fingerprints. Please re-open your pull request against opendbc at your convenience by using this command below. This will transform all changes under selfdrive/car/ to opendbc_repo/opendbc/car/. Make sure you have initialized submodules beforehand and have checked out the latest opendbc commit.

PR_NUMBER=33045
curl -L https://github.com/commaai/openpilot/pull/$PR_NUMBER.patch | sed -e 's/selfdrive\/car/opendbc_repo\/opendbc\/car/g' | git apply -v --reject

Simply replace the PR number with your own. Once done, add the files, fix any conflicts, and open a new PR. Alternatively, you may start a new PR from scratch if that is easier for you.