Skip to content

Conversation

@chrispypatt
Copy link

@chrispypatt chrispypatt commented Mar 22, 2025

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:

  • Add SecOC long safety tests
  • Confirm with SunnyPilot team this sticks to their idea of "minimal stock OP" changes
  • Sanity test in vehicle

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:

  • Adds safety tests for SecOC longitudinal control.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 22, 2025

Reviewer's Guide by Sourcery

This 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 update

sequenceDiagram
  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", ...)
Loading

Updated class diagram for CarController

classDiagram
  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."
Loading

File-Level Changes

Change Details Files
Adds SecOC longitudinal control by introducing new messages and modifying existing ones.
  • Adds ACC_CONTROL_2 message for longitudinal control.
  • Modifies ACC_CONTROL to set ACCEL_CMD to 0 when SecOC is enabled.
  • Adds message 0x750 for radar diagnostics.
  • Updates safety checks to validate ACC_CONTROL_2 and block ACC_CONTROL acceleration commands on SecOC cars.
  • Adds SecOCLong class to handle SecOC longitudinal control logic.
  • Adds SecOCLongCarController class to integrate SecOCLong into the car controller.
  • Adds tests for SecOC longitudinal control.
opendbc/safety/tests/test_toyota.py
opendbc/safety/safety/safety_toyota.h
opendbc/car/toyota/carcontroller.py
opendbc/car/toyota/interface.py
opendbc/car/toyota/toyotacan.py
opendbc/sunnypilot/car/toyota/secoc_long.py
opendbc/sunnypilot/car/toyota/tests/test_secoc_long_base.py
Updates test suite to include SecOC longitudinal control tests.
  • Adds TestToyotaSecOcSafety class to test SecOC safety features.
  • Modifies existing tests to account for the new ACC_CONTROL_2 message.
  • Adds tests for acceleration limits and message transmission.
  • Adds a base test class for SecOC longitudinal control tests.
opendbc/safety/tests/test_toyota.py
opendbc/sunnypilot/car/toyota/tests/test_secoc_long_base.py
Modifies the car controller to integrate SecOC longitudinal control.
  • Adds SecOCLongCarController to handle SecOC longitudinal control logic.
  • Updates the update method to call SecOCLongCarController.update.
  • Resets the SecOC message counter when the reset counter changes.
  • Updates the create_accel_command function to include the SECOC_LONG parameter.
opendbc/car/toyota/carcontroller.py
opendbc/car/toyota/toyotacan.py
opendbc/sunnypilot/car/toyota/secoc_long.py
Updates safety hooks to handle SecOC messages.
  • Adds 0x183 and 0x750 to the list of transmitted messages.
  • Updates relay malfunction addresses to include 0x183.
  • Updates forwarded message blacklist to include 0x183.
  • Modifies the toyota_tx_hook function to handle 0x183 messages.
  • Modifies the toyota_fwd_hook function to block 0x183 messages when not in stock longitudinal mode.
opendbc/safety/safety/safety_toyota.h

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@chrispypatt
Copy link
Author

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.

Copy link

@Discountchubbs Discountchubbs left a 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

@chrispypatt chrispypatt marked this pull request as ready for review March 26, 2025 02:12
@chrispypatt
Copy link
Author

@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 ecu? Or is there a better name than ecu_disabled for that test param?

Copy link

@Discountchubbs Discountchubbs left a 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

@chrispypatt
Copy link
Author

@Discountchubbs yeah I can do that. Would you like the route added to routes.py too? Or just documenting here as you said?

@Discountchubbs
Copy link

@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.

@chrispypatt
Copy link
Author

@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.

Screenshot 2025-04-10 at 1 10 57 AM Screenshot 2025-04-10 at 1 12 48 AM

@sunnyhaibin sunnyhaibin added the enhancement New feature or request label Apr 14, 2025
Copy link
Collaborator

@sunnyhaibin sunnyhaibin left a 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!

@sunnyhaibin sunnyhaibin merged commit ae51980 into sunnypilot:master-new Apr 14, 2025
7 checks passed
sunnyhaibin added a commit that referenced this pull request Apr 14, 2025
sunnyhaibin added a commit that referenced this pull request Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants