-
Notifications
You must be signed in to change notification settings - Fork 126
Toyota: SecOC Longitudinal Control #93
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
Conversation
Reviewer's Guide by SourceryThis PR introduces SecOC longitudinal control for Toyota vehicles. It includes changes to safety parameters, CAN message creation, and car controller logic to support the new control system. The implementation follows a modular pattern to minimize merge conflicts. Sequence diagram for SecOC longitudinal control updatesequenceDiagram
participant CarController
participant SecOCLongCarController
participant SecOCLong
participant CAN
CarController->SecOCLongCarController: update(CS, can_sends)
SecOCLongCarController->SecOCLong: update_car_state(CS)
SecOCLongCarController->SecOCLong: set_can_sends(can_sends)
alt CP.flags & ToyotaFlags.SECOC.value
SecOCLongCarController->SecOCLong: reset_counter()
end
CarController->toyotacan: create_accel_command(packer, pcm_accel_cmd, ...)
toyotacan->SecOCLong: update_accel_command(packer, values)
alt self.enabled
SecOCLong->SecOCLong: create_accel_2_command(packer, values["ACCEL_CMD"])
SecOCLong->CAN: packer.make_can_msg("ACC_CONTROL_2", ...)
SecOCLong->CAN: add_mac(secoc_key, ...)
SecOCLong->CarController: values["ACCEL_CMD"] = 0
end
CarController->CAN: packer.make_can_msg("ACC_CONTROL", ...)
Updated class diagram for CarControllerclassDiagram
class CarController {
-params: CarControllerParams
-last_torque: float
-last_angle: float
-secoc_prev_reset_counter: int
+__init__(dbc_names, CP, CP_SP)
+update(CC, CC_SP, CS, now_nanos)
}
class SecOCLongCarController {
-SECOC_LONG: SecOCLong
+__init__(CP: structs.CarParams)
+update(CS, can_sends)
}
CarController --|> SecOCLongCarController : inherits
CarController *-- CarControllerParams : has a
SecOCLongCarController *-- SecOCLong : has a
class SecOCLong {
-CP: structs.CarParams
-secoc_acc_message_counter: int
+__init__(CP: structs.CarParams)
+reset_counter()
+enabled()
+update_car_state(car_state)
+set_can_sends(can_sends)
+update_accel_command(packer, values)
+create_accel_2_command(packer, accel)
}
note for SecOCLong "This class handles SecOC longitudinal control logic."
note for CarController "This class integrates SecOCLongCarController to manage longitudinal control."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Tested in car. Seeing error "Cruise Fault: Restart the car to engage" on boot so I will need to deep dive what is getting tripped up. |
Discountchubbs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see requested changes in toyotacan.py
|
@sunnyhaibin Updated the PR with your suggestions. FYI, I followed your PR you shared to fix the tester present test. Is the radar ecu also referred to as |
Discountchubbs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! Sorry we have been swamped lately. Diving back into it, The tests are nice. Since you've done testing on road as well, do you have a route you ran through plotjuggler --can to provide objective evidence that everything works as intended? (aside from the tests (which are nice by the way)). For the route it can be screenshots of the graphs showing the secoc bit and proceeding accel commands aren't erratic
|
@Discountchubbs yeah I can do that. Would you like the route added to routes.py too? Or just documenting here as you said? |
|
@chrispypatt No need to add to routes.py for now since this isn't strictly a car port. Just post graphs here and in connect just have it set as preserved. |
|
@Discountchubbs Here's a couple of screenshots. Lmk if this is what you were looking for. The first is a holistic view of a route segment where I approach a car stopped at a stoplight and before stopping the light turns green. Hence the large negative accel dip. The graph to the right of accel in the screenshot show the secoc related bits. Authenticator looks random because that is the mac. The bottom two are counters. The second screenshot is a zoomed in view to show the secoc related signals.
|
sunnyhaibin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the PR!
This reverts commit ae51980.


This PR aims to add SecOC longitudinal control to master-new. It follows a similar modularized pattern as the Hyundai ESCC PR to minimize merge conflicts with the OP master.
This PR is still a work in progress. Items left include:
Please let me know if there is anything else the team wants me to look into before I mark this as ready for review.
Summary by Sourcery
Tests: