feat: add ResponseOnEventType enum#87
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adds UDS “ResponseOnEvent” type definitions and helpers to decode/encode the sub-function byte.
Changes:
- Introduces
ResponseOnEventType(andResponseOnEventTypeBytewrapper) forUdsCommand::ResponseOnEventsub-function values. - Adds
ResponseOnEventSubFunctionhelper struct for packing/unpacking bits 5:0 (event type) + bit 6 (storeEvent) with unit tests. - Exposes the new module via
src/uds/mod.rsand links it fromUdsCommand::ResponseOnEventdocs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/uds/response_on_event_type.rs |
New enum + sub-function byte packing/unpacking + tests. |
src/uds/mod.rs |
Wires the new module into the uds module and re-exports it. |
src/uds/commands.rs |
Adds doc link from UdsCommand::ResponseOnEvent to ResponseOnEventType. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| impl From<u8> for ResponseOnEventSubFunction { | ||
| fn from(byte: u8) -> Self { | ||
| Self { | ||
| event_type: ResponseOnEventTypeByte::from(byte & 0x3F), | ||
| store_event: (byte & 0x40) != 0, | ||
| } | ||
| } |
There was a problem hiding this comment.
impl From<u8> for ResponseOnEventSubFunction silently discards bit 7 (suppressPosRspMsgIndicationBit). If a caller passes a raw sub-function byte that still has bit 7 set, u8::from(ResponseOnEventSubFunction::from(byte)) will not roundtrip and will clear bit 7. Consider switching this conversion to TryFrom<u8> (rejecting bytes with bit 7 set) or otherwise enforcing/documenting that inputs must be pre-masked to 0x7F.
| fn from(v: ResponseOnEventSubFunction) -> Self { | ||
| let base: u8 = v.event_type.into(); | ||
| base | if v.store_event { 0x40 } else { 0x00 } | ||
| } |
There was a problem hiding this comment.
From<ResponseOnEventSubFunction> for u8 currently ORs store_event onto the raw event_type byte without masking it to bits 5:0. Because ResponseOnEventTypeByte is a ByteWrapper and can be Extended(u8), this can accidentally leak bits 6/7 into the encoded sub-function byte (contradicting the doc that only bits 5:0 are the event type). Mask base with 0x3F (and consider adding a unit test covering an Extended value with high bits set).
I'm not yet sure this is the right approach to this enum... TBD