Skip to content

Conversation

@dzid26
Copy link

@dzid26 dzid26 commented Nov 10, 2022

@wocsor
In my fork, I am using Ocelot-steering-dev branch.

Can we agree on a single interface before potentially more than two people start to use it?

Summary:

  • Added speed and temperature
  • Optimized signal sizes, added feasible scaling factors,
  • Added meaningful signal descriptions and states;
    • 4 basic operation modes: STEER_MODE 0 "Off" 1 "TorqueControl" 2 "AngleControl" 3 "SoftOff" ; SoftOff is a safe state that all steering actuators ideally should implement t satisfy some basic safety
    • Checksum algo specified 2771cbb. Implementation #1, #2
  • 2bytes signals aligned to 2 bytes per CANdb++: warnings
  • DRIVER_TORQUE moved to a separate message because almost certainly this wouldn't be ever sent by the motor actuator controller. I don't know if anyone uses it.

Cabana view:
image

STEERING_STATUS
image
STEERING_COMMAND
image

Consistency check report from CANdb++:
image

@wocsor
Copy link

wocsor commented Nov 10, 2022

i agree with the stance that we need a standardized interface. making sure the i/o are at least somewhat consistent across setups will be a decent undertaking, so let's get the CAN side out of the way first.

i suppose we don't need driver_torque, but we will need at least a one-bit message to communicate driver override somewhere. we follow J1850 for the spec, so it should be little-endian, and the checksum should be CRC8 with a poly of D5 at the top of the message (byte 0). aside from the algo, i see most of that is followed here. i have most of these reqs ported to STM32CubeIDE in another actuator project here:

https://github.com/wocsor/stm32f4_steer_actuator

i think we should define a few "diagnostic only" PIDs and move speed, current, temp, and other data in there.

@dzid26 dzid26 marked this pull request as ready for review November 10, 2022 18:26
@dzid26
Copy link
Author

dzid26 commented Nov 10, 2022

  • I would leave the temperature here. OP may want to do some derating action in the control loop and notify the user or inhibit engagement.
  • I can switch to crc8 to align the implementations

@dzid26
Copy link
Author

dzid26 commented Nov 10, 2022

I removed the driver torque message.
As for the override, my actuator will go to zero torque when STEER_MODE signal is set to 0 Off in STEERING_COMMAND (regardless of STEERING_TORQUE).

Is that what you mean by something else?

@dzid26
Copy link
Author

dzid26 commented Nov 10, 2022

CRC-8-SAE J1850 uses polynomial 0x1D, but you have it right in your c-code.
image
refrence

The only thing is J1850 doesn't specify how to present the data to the CRC algorithm. I am currently replacing the first byte with a Msg id instead of ignoring it. Here.

I looked at BMW now. They started using CRC in F-series and the algorithm uses 9 bytes. I figured that they feed CRC with 1st and 2nd bytes of msg id and then 7 data bytes.

This is the proper way to do it if we bother to use CRC at all, because it is easy for the controller to send correct data but with a wrong id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants