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

Subaru: POC for longitudinal control #25345

Closed
wants to merge 29 commits into from

Conversation

martinl
Copy link
Contributor

@martinl martinl commented Aug 3, 2022

Description POC for Subaru Global longitudinal control. For acceleration, there are 2 signals - cruise_throttle and cruise_rpm that are basically throttle and rpm target requests. Scaling values used in carcontroller are roughly modeled by plotting and observing stock acc signals and testing. Brake request is single linear signal with 0-400 scale.

Stock AEB/PCB is passed through.

The general idea for logitudinal control is that eyesight is not engaged by rewriting the feedback signals from car. openpilot generates the required signals for lateral and longitudinal control and dash display.

POC is usable and I have used it for testing the openpilot vision only performance from time to time

Verification Tested on 2018 Crosstrek

.gitmodules Outdated Show resolved Hide resolved
selfdrive/car/fw_versions.py Outdated Show resolved Hide resolved
selfdrive/car/subaru/interface.py Outdated Show resolved Hide resolved
@sshane sshane marked this pull request as draft August 12, 2022 03:22
@martinl
Copy link
Contributor Author

martinl commented Aug 17, 2022

I did some testing on subaru long branch and Eyesight also faults when rebased to latest master. I tracked down the commit using preserved subaru-long-c2 branch and fault starts after opendbc commit 859fea7 (CAN FD support in packer + parser).

Last currently working opendbc commit is 0ccc718

I will post relevant routes once they are uploaded

@martinl
Copy link
Contributor Author

martinl commented Aug 17, 2022

Working subaru-long-c2 route: 05bca04dfbdca165|2022-08-17--22-23-55
Eyesight fault on startup: 05bca04dfbdca165|2022-08-17--22-29-26

@adeebshihadeh
Copy link
Contributor

I did some testing on subaru long branch and Eyesight also faults when rebased to latest master. I tracked down the commit using preserved subaru-long-c2 branch and fault starts after opendbc commit 859fea7 (CAN FD support in packer + parser).

I would check if any signals are overlapping.

@martinl
Copy link
Contributor Author

martinl commented Aug 18, 2022

I tested the subaru dbc files

I did some testing on subaru long branch and Eyesight also faults when rebased to latest master. I tracked down the commit using preserved subaru-long-c2 branch and fault starts after opendbc commit 859fea7 (CAN FD support in packer + parser).

I would check if any signals are overlapping.

I tested loading subaru dbc files using cantools and there are no overlapping signals. I created one signals overlap for testing and cantools reported it correctly.

I also enabled DEBUG in opendbc/can/common.h but did not find any clues related to can signals or checksums being wrong

@martinl
Copy link
Contributor Author

martinl commented Aug 18, 2022

with latest version / feature-subaru-long branch I get during startup

opendbc/can/parser.cc: 0x3AC MISSING
opendbc/can/parser.cc: 0x119 MISSING
opendbc/can/parser.cc: 0x3AC MISSING
opendbc/can/parser.cc: 0x3AC MISSING
opendbc/can/parser.cc: 0x3AC MISSING
opendbc/can/parser.cc: 0x3AC MISSING
opendbc/can/parser.cc: 0x3AC MISSING
opendbc/can/parser.cc: 0x322 MISSING
opendbc/can/parser.cc: 0x321 MISSING
opendbc/can/parser.cc: 0x322 MISSING
opendbc/can/parser.cc: 0x321 MISSING
opendbc/can/parser.cc: 0x322 MISSING
opendbc/can/parser.cc: 0x321 MISSING
opendbc/can/parser.cc: 0x322 MISSING
opendbc/can/parser.cc: 0x321 MISSING
opendbc/can/parser.cc: 0x322 MISSING
opendbc/can/parser.cc: 0x321 MISSING
opendbc/can/parser.cc: 0x3AC MISSING
opendbc/can/parser.cc: 0x3AC MISSING
opendbc/can/parser.cc: 0x3AC MISSING

I tried to comment out can_valid checks in opendbc/can/parser.cc (can_valid is always true) and set all frequency checks in carstate to 1 but eyesight still faulted.

Are there any other debug options to enable that might be useful to finding the cause?

@martinl
Copy link
Contributor Author

martinl commented Sep 4, 2022

I made some progress narrowing down the issue - Eyesight does not fault if I allow forwarding of Brake_Status and CruiseControl messages in panda safety. I added debug output to subarucan to print the sent Brake_Status and CruiseControl messages

working, subaru-long-c2 branch:

send cruisecontrol
[576, 0, b'\xd4\x05\x00\xa46\x00\x08\xab', 2]
send brake_status
[316, 0, b'\x0f/\x00\x00\x00\x00@c', 2]
send brake_status
[316, 0, b'\x00 \x00\x00\x00\x00@c', 2]
send brake_status
[316, 0, b'\x01!\x00\x00\x00\x00@c', 2]
send cruisecontrol
[576, 0, b'\xd6\x06\x00\xa46\x00\x08\xac', 2]
send brake_status
[316, 0, b'\x02"\x00\x00\x00\x00@c', 2]
selfdrive/boardd/boardd.cc: got 736 bytes CarParams
selfdrive/boardd/boardd.cc: panda 0: setting safety model: 11, param: 0, alternative experience: 1
send brake_status
[316, 0, b'\x03#\x00\x00\x00\x00@c', 2]
send brake_status
[316, 0, b'\x04$\x00\x00\x00\x00@c', 2]
send cruisecontrol
[576, 0, b'\xd7\x07\x00\xa46\x00\x08\xac', 2]
send brake_status
[316, 0, b'\x05%\x00\x00\x00\x00@c', 2]
send brake_status
[316, 0, b'\x06&\x00\x00\x00\x00@c', 2]

not working, subaru-feature-long:

send cruisecontrol
[576, 0, b'\xa8\n\x00\xa0\x06\x00\x08\xae', 2]
send brake_status
[316, 0, b'\x03#@\x03\x00\x00\x00`', 2]
send brake_status
[316, 0, b'\x04$@\x03\x00\x00\x00`', 2]
send brake_status
[316, 0, b'\x05%@\x03\x00\x00\x00`', 2]
selfdrive/boardd/boardd.cc: got 1408 bytes CarParams
selfdrive/boardd/boardd.cc: panda 0: setting safety model: 11, param: 2, alternative experience: 1
send brake_status
[316, 0, b'\x06&@\x03\x00\x00\x00`', 2]
send brake_status
[316, 0, b"\x07'@\x03\x00\x00\x00`", 2]
send brake_status
[316, 0, b'\x08(@\x03\x00\x00\x00`', 2]
send cruisecontrol
[576, 0, b'\xab\x0c\x00\xa0\x06\x00\x08\xaf', 2]
send brake_status
[316, 0, b'\t)@\x03\x00\x00\x00`', 2]
send brake_status
[316, 0, b'\n*@\x03\x00\x00\x00`', 2]
send cruisecontrol
[576, 0, b'\xad\r\x00\xa0\x06\x00\x08\xb0', 2]

Eyesight faults when the panda safety mode is set

@martinl
Copy link
Contributor Author

martinl commented Sep 7, 2022

I split the 46bit Brake_Status Signal1 signal to two 23bit signals in test branch and that fixed eyesight fault. I'll see if I can fix opendbc large values issue but I may need help with that.

@adeebshihadeh
Copy link
Contributor

CAN Parser bug fixed and opendbc has been bumped on master with the fix.

@martinl
Copy link
Contributor Author

martinl commented Sep 8, 2022

Thanks. I merged the latest master and it's working with Crosstrek. I'll look into why gen2 car interface tests are failing, process replay failure is expected

selfdrive/car/subaru/carcontroller.py Outdated Show resolved Hide resolved
selfdrive/car/subaru/carcontroller.py Outdated Show resolved Hide resolved
selfdrive/car/subaru/interface.py Outdated Show resolved Hide resolved
selfdrive/car/subaru/values.py Show resolved Hide resolved
@martinl
Copy link
Contributor Author

martinl commented Oct 2, 2022

not directly relevant but I had to add the if sys.platform.startswith("linux"): conditions back in common/realtime.py because on macos pre-commit checks fail with mypy errors

common/realtime.py:35: error: Module has no attribute "sched_setscheduler"
common/realtime.py:40: error: Module has no attribute "sched_setaffinity"

@martinl
Copy link
Contributor Author

martinl commented Oct 16, 2022

I moved the dev environment to ubuntu and reverted linux platform checks. Also moved ES_Status and ES_Brake to can1 for GEN2, so all route tests are passing now.

@martinl martinl force-pushed the feature-subaru-long branch from e944a57 to dfc72e1 Compare March 5, 2023 10:50
@martinl martinl mentioned this pull request Apr 27, 2023
@martinl
Copy link
Contributor Author

martinl commented Apr 27, 2023

Stock AEB PR #28052

@jnewb1 jnewb1 mentioned this pull request Jun 13, 2023
6 tasks
("COUNTER", "ES_Brake"),
("Signal1", "ES_Brake"),
("Brake_Pressure", "ES_Brake"),
("Signal2", "ES_Brake"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES_Status.Signal2 no longer exists, and kills controlsd when the checks run.

This is likely ES_Brake.AEB_Status now... which already exists on line 132.

Suggested change
("Signal2", "ES_Brake"),


("COUNTER", "ES_Brake"),
("Signal1", "ES_Brake"),
("Brake_Pressure", "ES_Brake"),
Copy link
Contributor

@michaelhonan michaelhonan Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES_Brake.Brake_Pressure check already exists above on line 133

@martinl
Copy link
Contributor Author

martinl commented Jul 5, 2023

the required changes are already implemented in feature-subaru-long-2023-07-02 branch, I will merge soon after testing

@martinl
Copy link
Contributor Author

martinl commented Aug 13, 2023

Superseded by #28872 - closing this

@martinl martinl closed this Aug 13, 2023
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.

4 participants