Skip to content

Add can custom timing - #166

Merged
ivmarkov merged 1 commit into
esp-rs:masterfrom
elotom:add-can-custom-timing
Nov 18, 2022
Merged

Add can custom timing#166
ivmarkov merged 1 commit into
esp-rs:masterfrom
elotom:add-can-custom-timing

Conversation

@elotom

@elotom elotom commented Nov 17, 2022

Copy link
Copy Markdown
Contributor

It would be nice if we could set custom CAN timings. I'm working on a project which uses a non-standard one.

This change requires deriving PartialEq and Eq for twai_timing_config_t. I'll make an additional pull request for esp-idf-sys to fix the compilation issue.

Comment thread src/can.rs Outdated
triple_sampling: false,
..Default::default()
},
Timing::Custom(twai_timing_config_t) => twai_timing_config_t,

@ivmarkov ivmarkov Nov 18, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rather than exposing the ESP IDF C structure twai_timing_config_t can you implement a custom Rust structure - say - CustomTimingConfig and then also conversion implement code (might be just impl From<CustomTimingConfig> for twai_timing_config_t)?

Alternatively, and perhaps even better - you can implement the custom field members directly on the Custom variant of the Timing enum, and then impl From<Timing> for twai_timing_config_t?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, I added the custom field members directly on the Custom variant.

@elotom
elotom force-pushed the add-can-custom-timing branch from 3af7c7e to eec65c3 Compare November 18, 2022 12:14
@elotom
elotom force-pushed the add-can-custom-timing branch from eec65c3 to f22556c Compare November 18, 2022 12:17
@ivmarkov

Copy link
Copy Markdown
Collaborator

Thanks!

@ivmarkov
ivmarkov merged commit 0124676 into esp-rs:master Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants