feat(canbus): update protocol of vehicle ch, devkit and neolix_edu #66
Conversation
Signed-off-by: Pride Leong <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR updates the CAN bus protocol definitions for three vehicle types (ch, devkit, and neolix_edu), adding support for VIN retrieval, enhanced vehicle mode commands, improved battery monitoring, and additional sensor data reporting.
Key Changes:
- Added VIN (Vehicle Identification Number) request/response protocols for all three vehicle types
- Enhanced vehicle mode commands with turn signal and drive mode controls
- Extended battery management system reporting with temperature and fault detection
- Added new brake state enums and rear steering angle reporting
Reviewed Changes
Copilot reviewed 91 out of 91 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
modules/common_msgs/chassis_msgs/*.proto |
Updated protocol buffer definitions with new messages and enums for VIN, vehicle modes, and sensors |
modules/canbus/vehicle/*/protocol/vin_resp*.{h,cc} |
Implemented VIN response parsing across all vehicle types |
modules/canbus/vehicle/*/protocol/vehicle_mode_command_*.{h,cc} |
Added vehicle mode command protocols for turn signals and VIN requests |
modules/canbus/vehicle/devkit/devkit_controller.cc |
Integrated VIN retrieval, sonar data, bumper events, and emergency brake logic |
modules/canbus/vehicle/neolix_edu/neolix_edu_controller.cc |
Added bumper event detection based on brake state |
modules/canbus/vehicle/vehicle_controller.{h,cc} |
Added VerifyID() virtual method for VIN verification |
modules/canbus/vehicle/*/BUILD |
Added build targets for new protocol files and CANBUS_COPTS flags |
modules/canbus/vehicle/*/protocol/*_test.cc |
Updated test cases to match new protocol field values |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| int32_t x = t0.get_byte(0, 8); | ||
|
|
||
| std::string ret = ""; | ||
| ret += x; |
There was a problem hiding this comment.
The string concatenation using ret += x where x is an integer value (0-255) will append the integer as a character, not convert it to its string representation. This could result in non-printable or incorrect characters in the VIN string. Consider using proper integer-to-string conversion like ret += std::to_string(x) or if the intent is to interpret the integer as an ASCII character, this should be documented.
| int32_t x = t0.get_byte(0, 8); | ||
|
|
||
| std::string ret = ""; | ||
| ret += x; |
There was a problem hiding this comment.
Same issue: ret += x where x is an integer will append the integer as a character, not its string representation. This pattern appears throughout all VIN parsing functions and needs correction.
| return ret; | ||
| } | ||
|
|
||
| // config detail: {'bit': 32, 'description': 'describle the vehicle AEB mode |
There was a problem hiding this comment.
Typo in comment: "describle" should be "describe".
| // to do(ALL):check your vehicle type, confirm your sonar position because of | ||
| // every vhechle has different sonars assembly. |
There was a problem hiding this comment.
Typo in comment: "vhechle" should be "vehicle".
|
|
||
| bool DevkitController::CheckVin() { | ||
| if (chassis_.vehicle_id().vin().size() == 17) { | ||
| AINFO << "Vin check success! Vehicel vin is " |
There was a problem hiding this comment.
Typo in comment: "Vehicel" should be "Vehicle".
| double Throttlecommand100::speed_target(const std::uint8_t* bytes, | ||
| int32_t length) const { | ||
| Byte t0(bytes + 5); | ||
| int32_t x = t0.get_byte(0, 8); | ||
|
|
||
| Byte t1(bytes + 6); | ||
| int32_t t = t1.get_byte(6, 2); | ||
| x <<= 8; | ||
| x |= t; | ||
|
|
||
| double ret = x * 0.100000; | ||
| return ret; |
There was a problem hiding this comment.
The speed_target method incorrectly uses a multiplier of 0.100000 (precision 0.1) when the field definition specifies a precision of 0.01. This will result in incorrect speed values being read.
update vehicle protocol