hkg: Add ESCC (Enhanced Smart Cruise Control) support #70
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request introduces support for Enhanced Smart Cruise Control (ESCC) in Hyundai vehicles. It includes changes to the main control logic, safety configurations, and build process to accommodate the new ESCC functionality. Additionally, a test script for flashing ESCC firmware over CAN has been added. Sequence diagram for ESCC message handlingsequenceDiagram
participant Car as Car CAN Bus
participant Radar as Radar CAN Bus
participant ESCC as ESCC Module
participant SP as SunnyPilot
Note over ESCC: Initialize ESCC message structure
Radar->>ESCC: SCC11 (0x420)
activate ESCC
ESCC->>ESCC: Update radar points data
ESCC->>Car: Send ESCC message (0x2AB)
deactivate ESCC
Radar->>ESCC: SCC12 (0x421)
activate ESCC
ESCC->>ESCC: Update AEB data
ESCC->>Car: Send ESCC message (0x2AB)
deactivate ESCC
Radar->>ESCC: FCA11 (0x38D)
activate ESCC
ESCC->>ESCC: Update FCA data
ESCC->>Car: Send ESCC message (0x2AB)
deactivate ESCC
Class diagram for ESCC message structureclassDiagram
class ESCC_Msg {
+uint8_t fca_cmd_act
+uint8_t aeb_cmd_act
+uint8_t cf_vsm_warn_fca11
+uint8_t cf_vsm_warn_scc12
+uint8_t cf_vsm_deccmdact_scc12
+uint8_t cf_vsm_deccmdact_fca11
+uint8_t cr_vsm_deccmd_scc12
+uint8_t cr_vsm_deccmd_fca11
+uint8_t obj_valid
+uint8_t acc_objstatus
+uint8_t acc_obj_lat_pos_1
+uint8_t acc_obj_lat_pos_2
+uint8_t acc_obj_dist_1
+uint8_t acc_obj_dist_2
+uint8_t acc_obj_rel_spd_1
+uint8_t acc_obj_rel_spd_2
}
class safety_hooks {
+init()
+rx()
+tx()
+fwd()
+get_counter()
+get_checksum()
+compute_checksum()
}
safety_hooks <|-- hyundai_escc_hooks : implements
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Add initial README for ESCC board This commit introduces a new README for the ESCC board detailing the pinout configurations and connections between the Molex connector and IONIQ harness and radar components. The documentation provides a clear, structured guide to assist with proper wiring and setup. Improving comments for clarity and explicitness IDE is behaving now. Wd don't need this hack anymore Add ESCC build option and refine ESCC specific code Introduce --escc option in SCons to conditionally build ESCC firmware. Update build_project to support CPPDEFINES and modify main.c to handle ESCC-specific safety modes and behaviors more cleanly. Remove unnecessary send_escc_msg calls in Hyundai safety code The updates in the code remove redundant send_escc_msg calls in the Hyundai safety code. We don't really need to send the FCA or AEB data through the ESCC packet because we don't block it at all. So we don't really need to "package" it anymore on the ESCC message Add send_escc_msg call in case blocks The send_escc_msg function is now called within the individual case blocks for SCC11, SCC12, and FCA11, enhancing code readability. Before, it was called on any message that the radar sends, but this strategy was inefficient as it was piggybacking on their frequencies. Refactor `escc_tx_hook` function in Hyundai safety code The commented-out `escc_tx_hook` function in Hyundai ESCC safety code has been refactored and is now in use again, replacing the formerly used `alloutput_tx_hook`. Debugging functionality has been preserved and conditions for when the function is unused have also been handled. The resulting code is now cleaner and more efficient. Add ESCC firmware and build files, and safety mode for Hyundai ESCC This commit introduces ESCC firmware and the corresponding build files. We have also defined a new safety mode for Hyundai ESCC. The necessary scripts for flashing and recovery have been added, alongside the gitignore file for the ESCC board. Additionally, we have implemented new message processing and forwarding logic to handle ESCC specific messages in main and safety files.
devtekve
force-pushed
the
master-new-escc-reworked
branch
from
3cf59d6 to
afd5d9f
Compare
devtekve
changed the title
devtekve
changed the title
devtekve
changed the title
|
@sourcery-ai reviewer's guide again (ESCC is Enhanced Smart Cruise Control) |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @devtekve - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider documenting the ESCC message protocol and bit layout in safety_hyundai_escc.h to make the code more maintainable
- The debug logging could be made more consistent - consider using a structured logging approach rather than scattered print statements
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
devtekve
force-pushed
the
master-new-escc-reworked
branch
from
6db7693 to
f62a079
Compare
There was a problem hiding this comment.
Hey @devtekve - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| #define DEVNULL_BUS (-1) | ||
| #define CAR_BUS 0 | ||
| #define RADAR_BUS 2 |
There was a problem hiding this comment.
issue (complexity): Consider extracting the message parsing logic into separate functions to improve maintainability and testability.
The message handling could be simplified by extracting parsing logic into message-specific functions. This would improve maintainability while keeping the core logic intact:
typedef struct {
uint8_t obj_valid;
uint8_t acc_objstatus;
// ... other fields
} SCC11_Data;
static SCC11_Data parse_scc11(const CANPacket_t* msg) {
SCC11_Data data = {0};
data.obj_valid = (GET_BYTE(msg, 2) & 0x1U);
data.acc_objstatus = ((GET_BYTE(msg, 2) >> 6) & 0x3U);
// ... parse other fields
return data;
}
static void escc_rx_hook(const CANPacket_t* to_push) {
if (bus == RADAR_BUS && (is_scc_msg || is_fca_msg)) {
switch (addr) {
case 0x420: {
SCC11_Data data = parse_scc11(to_push);
escc.obj_valid = data.obj_valid;
escc.acc_objstatus = data.acc_objstatus;
// ... update other fields
send_escc_msg(&escc, CAR_BUS);
break;
}
// ... other cases
}
}
}This approach:
- Separates message parsing from state updates
- Makes the code more testable
- Reduces cognitive load by grouping related fields
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@sourcery-ai
supersedes #28
Summary by Sourcery
Add support for Enhanced Smart Cruise Control (ESCC) on Hyundai vehicles. This includes sending ESCC messages over CAN.
New Features:
Tests: