Skoda unsupported connector footnote; note J533 alternative isn't available on all cars

[belm0]

cc: @jyoung8607 @vanillagorillaa

see https://discord.com/channels/469524606043160576/534359517836607488/999917791262031912

[incognitojam]

You will need to add the footnote programmatically instead, see here where the others were defined: https://github.com/commaai/openpilot/blob/master/selfdrive/car/volkswagen/values.py#L99

[belm0]

You will need to add the footnote programmatically instead, see here where the others were defined: https://github.com/commaai/openpilot/blob/master/selfdrive/car/volkswagen/values.py#L99

Thank you-- you mean that CARS.md is generated.

By the way, I noticed that multiple footnotes is not rendered correctly (only the first footnote is referenced). E.g.:

VWCarInfo("Volkswagen Passat 2015-22", footnotes=[Footnote.VW_HARNESS, Footnote.PASSAT, Footnote.VW_VARIANT], harness=Harness.j533),

renders as

[belm0]

I'm not sure if the footnote isn't needed on all SEAT and Audi entries as well. Wait until they are confirmed in the wild?

[belm0]

(At the moment I don't have an openpilot dev environment with proper dependencies to run selfdrive/car/docs.py.)

[belm0]

@sshane thank you for running docs.py, though I think I need to rebase to master first to get the multiple-footnote fix, otherwise this PR would overwrite the generated code

[belm0]

rebased-- this PR will need another run of docs.py before merge

[jyoung8607]

Footnote.VW_HARNESS is for MY2021/2022 and beyond vehicles where the required harness changes due to introduction of the MFK 3.0 camera. It's not brand specific. I've been adding the footnote references as MY2022 support comes up. It's Volkswagen-heavy right now because I started a sweep recently (#25123) to extend supported model-years. It's research-intensive and I haven't gotten to the other brands yet.

You're correct to note some vehicles have popped up that don't have the traditional J533 connector anymore. The affected cars appear to be MY2022 and forward MQB-A0 with a new combined BCM/CAN gateway, but that is based on a small sample size. I'm not exactly sure what to say about that in the footnote, and have really been hoping @vanillagorillaa's work on a new MFK 3.0 connector pays off soon.

[belm0]

Footnote.VW_HARNESS is for MY2021/2022 and beyond vehicles where the required harness changes due to introduction of the MFK 3.0 camera. It's not brand specific. I've been adding the footnote references as MY2022 support comes up.

Do you mean this footnote shouldn't be added to all Skoda models?

Since MFK 3.0 is confirmed in one model, it seems highly likely to be adopted by other models. The footnote is and asking users to verify the connector before purchasing a harness. Is there harm in conservatively marking all models of the brand?

[jyoung8607]

Do you mean this footnote shouldn't be added to all Skoda models?

I mean it should be added as we bring MY2021/2022 onboard, or update the docs with research showing we can. Technically your PR #25271 would have been the time to add it to Karoq, but I didn't want to hold it up for revision given that you had this PR open on similar grounds.

[belm0]

I mean it should be added as we bring MY2021/2022 onboard, or update the docs with research showing we can. Technically your PR #25271 would have been the time to add it to Karoq, but I didn't want to hold it up for revision given that you had this PR open on similar grounds.

@jyoung8607 will you OK this PR if the footnote is limited to Karoq only?

You're correct to note some vehicles have popped up that don't have the traditional J533 connector anymore. The affected cars appear to be MY2022 and forward MQB-A0 with a new combined BCM/CAN gateway, but that is based on a small sample size. I'm not exactly sure what to say about that in the footnote

I'd really like to disclose something in the footnote about the possibility of no J533 (however small). It's not a great user experience to be promised a workaround and find out after purchasing the harness that you don't have the port.

[sshane]

Is this PR ready to merge?

[jyoung8607]

It's close enough. I wish I had a better definition of which newer cars have combined BCM and CAN gateway hardware (I think it's the MQB-A0 compacts) but this is better than no note at all.

[belm0]

Is this PR ready to merge?

yes, please run docs.py and merge