GM Switch to AcceleratorPedal2 for pedal pressed

[JMPZ11]

Updated GM safety code and tests to use AcceleratorPedal2 (452) rather than AcceleratorPedal (417) for detecting gas pressed.

The values for the pedal in the two messages are identical, and AcceleratorPedal is not present on the 2020 Silverado. 452 is present in all the existing fingerprints that also contain 417, and they were* both required in OP.

Corresponds with OP PR commaai/openpilot#23280

[adeebshihadeh]

In the openpilot PR, we’ll need a route with rlogs uploaded with each car this affects for verification.

[JMPZ11]

In the openpilot PR, we’ll need a route with rlogs uploaded with each car this affects for verification.

That would theoretically be all the GM vehicles - do you need new routes for every GM car? We cannot validate via existing routes?

[adeebshihadeh]

That would theoretically be all the GM vehicles - do you need new routes for every GM car? We cannot validate via existing routes?

Old routes are fine as long as they have rlogs still and sufficient data to validate this new signal. Ideally, I’d run something on thousands of GM segments, but most GM models don’t have many users and don’t upload rlogs to us.

[JMPZ11]

That would theoretically be all the GM vehicles - do you need new routes for every GM car? We cannot validate via existing routes?

Old routes are fine as long as they have rlogs still and sufficient data to validate this new signal. Ideally, I’d run something on thousands of GM segments, but most GM models don’t have many users and don’t upload rlogs to us.

I doubt I will be able to adequately validate that.

Ok, as an alternative, I can have the Panda and OP only switch to AcceleratorPedal2 when necessary. Does that sound better? If so, should I update these PRs or create new ones?

[jyoung8607]

This PR would affect seven cars. Four of them have CI test routes, and PlotJuggler shows both signals behaving identically. The remaining three are on the naughty list, and have been there for a very long time:

• GM.CADILLAC_ATS
• GM.HOLDEN_ASTRA
• GM.MALIBU

On one hand, we never want to miss a gas-press signal for safety reasons, so we do need positive confirmation from all cars this PR would affect. On the other hand, the GM port needs some TLC and needs to support future models, we have @JasonJShuler here willing to step up, and the last number I saw for total active GM vehicles on stock openpilot (let alone the naughty list) was best characterized as "not many".

So... I'd suggest that Jason put out a call out in #gm (and perhaps other community Discords) for active owners of the three vehicles above, and as we get responses, get test routes uploaded, get them off the naughty list, and confirm the accelerator signal behavior. If we can't find owners after a few days, I propose we flag those cars as dashcam-only and let the owners find us, so that Jason can move forward with development on current-model-year vehicles with active owners.

GM.ACADIA 7cc2a8365b4dd8a9|2018-12-02--12-10-44--1:

GM.BUICK_REGAL aa20e335f61ba898|2019-02-05--16-59-04--0:

GM.ESCALADE_ESV 46460f0da08e621e|2021-10-26--07-21-46--0:

GM.VOLT c950e28c26b5b168|2018-05-30--22-03-41--1:

[adeebshihadeh]

Thanks for the review and plots, @jyoung8607! That's exactly what I was gonna say. I'm fine with moving cars we can't verify to dashcam only once we've determined no one is using them. Perhaps we'll make a special startup alert for them too.

In the future @JasonJShuler (and any other contributor reading this), this is the kind of diligence and verification that I would expect for a change like this. Let's wait a bit to hear back about getting test routes from the missing cars.

[jyoung8607]

Jason put out a renewed call for test routes earlier today. That said, looking back, Shane put out the same call back in March of this year, and Chris put out the same call back in July 2020. The only response we got was from "the guy with the Cadillac ATS" (yeah, it's like that) and the substance of his response was his ASCM/topology is too modified to easily run stock openpilot.

So I'm fully anticipating dashcam mode. We can leave a comment next to it, avowing our dearest wish and fondest desire to remove that flag forthwith upon receipt of a usable CI test route. Maybe there's some quiet owners out there in downstream forkland that will notice the speedbump and check in.

Perhaps we'll make a special startup alert for them too.

Agreed, it would be nice to have a startup alert for "We still love you, but we need you to come check in with the community and help us verify some new safety checks before you'll be able to engage". Not quite sure how to wordsmith that down to the available C2/C3 display space, but something to think about over the holiday.

[jyoung8607]

Did you still want to put those three cars in dashcam mode? The guy with the Cadillac ATS reports that he sold it months ago, and nothing but silence from the other two TMK.

[adeebshihadeh]

Thanks for the reminder. commaai/openpilot#23423