Skip to content

Logging of all received frames #6097

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.

Sign up for GitHub

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

Open
eWuPi wants to merge 1 commit into
base: current
Choose a base branch
from
Open

Logging of all received frames #6097

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions content/components/canbus/_index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,16 @@ canbus:
- lambda: |-
std::string b(x.begin(), x.end());
ESP_LOGD("CAN standard ID 0x123", "%s", &b[0]);

- can_id: 0
can_id_mask: 0x000
use_extended_id: true
then:
Comment on lines +241 to +244
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This on_frame entry is described (in the PR title/description) as logging all received frames, but with use_extended_id: true it will only match extended-ID frames. If the intent is truly “all frames”, either omit use_extended_id and add separate triggers for standard vs extended IDs (as needed), or clarify in the example text that it only logs extended frames.

Copilot uses AI. Check for mistakes.
- lambda: |-
ESP_LOGI("CAN", "can_id: 0x%03X", can_id);
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this trigger can run for every received frame, logging at ESP_LOGI may spam logs and impact performance on busy buses. Consider using ESP_LOGD (and/or adding a short note that this is verbose logging intended for debugging).

Suggested change
ESP_LOGI("CAN", "can_id: 0x%03X", can_id);
ESP_LOGD("CAN", "can_id: 0x%03X", can_id);

Copilot uses AI. Check for mistakes.
if (x.size() > 0) {
ESP_LOGD("CAN", "First data: 0x%02X", x[0]);
Comment on lines +246 to +248
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added logging claims to include “frame ID and data”, but it only logs the CAN ID and (conditionally) the first payload byte. Consider logging the full payload (e.g., using the existing docs pattern with format_hex_pretty(x)), so the example actually shows the full received data.

Suggested change
ESP_LOGI("CAN", "can_id: 0x%03X", can_id);
if (x.size() > 0) {
ESP_LOGD("CAN", "First data: 0x%02X", x[0]);
ESP_LOGI("CAN", "Frame received - can_id: 0x%03X, data: %s", can_id, format_hex_pretty(x).c_str());

Copilot uses AI. Check for mistakes.

Comment on lines +245 to +249
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix the missing closing brace in the lambda example.

The if (x.size() > 0) { block is not closed in the snippet, which makes the example invalid if copied as-is.

🛠️ Proposed fix 
         - lambda: |-
             ESP_LOGI("CAN", "can_id: 0x%03X", can_id);
             if (x.size() > 0) {
               ESP_LOGD("CAN", "First data: 0x%02X", x[0]);
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- lambda: |-
ESP_LOGI("CAN", "can_id: 0x%03X", can_id);
if (x.size() > 0) {
ESP_LOGD("CAN", "First data: 0x%02X", x[0]);
- lambda: |-
ESP_LOGI("CAN", "can_id: 0x%03X", can_id);
if (x.size() > 0) {
ESP_LOGD("CAN", "First data: 0x%02X", x[0]);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@content/components/canbus/_index.md` around lines 245 - 249, The lambda
example in the snippet leaves the if-block unclosed; add the missing closing
brace to properly terminate the if (x.size() > 0) { block so the lambda is
valid. Locate the lambda containing ESP_LOGI("CAN", "can_id: 0x%03X", can_id);
and ESP_LOGD("CAN", "First data: 0x%02X", x[0]); and append the corresponding
"}" to close the if block (and ensure the lambda itself remains syntactically
correct).

Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The YAML example’s lambda has an opening if (x.size() > 0) { but no closing } before the code block ends, leaving the snippet syntactically incomplete (and likely to confuse readers/copy-paste). Close the if block (and ensure the lambda body is complete) before the fenced code block terminates.

Suggested change
}

Copilot uses AI. Check for mistakes.
```

## Binary Sensor Example
Expand Down
Loading