feat(isotp): Add thread safety, callbacks, and extended ID support (IEC-370)#563
Conversation
| // Set user argument for TWAI operations | ||
| // Pre-allocate ISR-safe receive frame buffer to avoid dynamic allocation in interrupt context. | ||
| // This buffer is reused for each incoming TWAI frame received in the ISR. | ||
| memset(&isotp->isr_rx_frame_buffer, 0, sizeof(esp_isotp_frame_t)); |
Check warning
Code scanning / clang-tidy
Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling] Warning
abf8778 to
3e62e8b
Compare
3e62e8b to
77d10b1
Compare
| ESP_RETURN_ON_FALSE_ISR(tx_frame != NULL, ISOTP_RET_ERROR, TAG, "No available frames in pool"); | ||
|
|
||
| // Initialize TWAI frame header and copy payload data into embedded buffer. | ||
| memset(&tx_frame->frame, 0, sizeof(twai_frame_t)); |
Check warning
Code scanning / clang-tidy
Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling] Warning
There was a problem hiding this comment.
Pull Request Overview
This PR transforms the esp_isotp component from a basic polling implementation to a production-ready ISO-TP library with thread safety, event-driven callbacks, and extended feature support.
- Adds thread safety with spinlock protection and ISR-safe send functions
- Introduces event-driven callbacks (rx_callback, tx_callback) for asynchronous notifications
- Implements 29-bit extended ID support with automatic format detection
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| esp_isotp/src/esp_isotp.c | Core implementation with thread safety, memory management, callback system, and extended ID support |
| esp_isotp/inc/esp_isotp.h | API enhancements with callback types, updated function signatures, and comprehensive documentation |
| esp_isotp/README.md | Updated documentation with usage examples, extended ID configuration, and error handling guidance |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
77d10b1 to
2eccc39
Compare
b37080a to
fc54250
Compare
| ESP_RETURN_ON_FALSE_ISR(size <= 8, ISOTP_RET_ERROR, TAG, "Invalid TWAI frame size"); | ||
|
|
||
| // Copy payload into the embedded buffer to ensure data lifetime during async transmission. | ||
| memcpy(tx_frame->data_payload, data, size); |
Check warning
Code scanning / clang-tidy
Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling] Warning
5a49f7b to
03db9f8
Compare
…d safety features
…ynchronous sending by pre-alloc a list tx_buffers. Now the esp_isotp_send(_with_id) supports interrupt context calls
03db9f8 to
50cfb28
Compare
50cfb28 to
9bd1a9f
Compare
9bd1a9f to
a8920b1
Compare
suda-morris
left a comment
There was a problem hiding this comment.
LGTM. The using of the link_lock to protect all isotpc API is a little bit overkill to me.
a8920b1 to
223dcd4
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
223dcd4 to
49e973f
Compare
| tx_frame->frame.header.ide = is_extended_id(arbitration_id); // Extended (29-bit) vs Standard (11-bit) ID | ||
|
|
||
| // Size validation - TWAI frames are max 8 bytes by protocol | ||
| ESP_RETURN_ON_FALSE_ISR(size <= 8, ISOTP_RET_ERROR, TAG, "Invalid TWAI frame size"); |
There was a problem hiding this comment.
this check should be moved to the begging of the function. @eternal-echo
Overview
This PR enhances the
esp_isotpcomponent with thread safety, event-driven callbacks, and extended feature support, transforming it from a basic polling implementation to a production-ready ISO-TP communication library.Key Features
Thread Safety & ISR Support
esp_isotp_send()andesp_isotp_send_with_id()can be called from interrupt contextportMUXlocks to prevent race conditionsEvent-Driven Callbacks
rx_callbackandtx_callbackin configurationExtended Features
use_extended_idconfiguration optionesp_isotp_send_with_id()function for runtime ID overrideesp_err_treturn codesAPI Changes
New Configuration Options
New Functions
esp_err_t esp_isotp_send_with_id(esp_isotp_handle_t handle, uint32_t id, const uint8_t *data, uint32_t size)Breaking Changes
None. This enhancement maintains full backward compatibility.
Files Modified
src/esp_isotp.c: Core implementation with memory management and thread safetyinc/esp_isotp.h: API enhancements and documentationREADME.md: Updated documentation with usage examplesNotes
esp_isotp_poll()must still be called periodically for multi-frame message handlingChecklist
urlfield defined