Skip to content
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.

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

Add support for dbcs with extended multiplex messages #8

Merged
merged 2 commits into from
Jun 22, 2022
Merged

Add support for dbcs with extended multiplex messages #8

merged 2 commits into from
Jun 22, 2022

Conversation

pbert519
Copy link

Parsing a DBC file with extendend multiplexing (multiple and possibly nested multiplexors per message) results currently in a partly parsed file.
Extended Multiplexing is described here: https://cdn.vector.com/cms/content/know-how/_application-notes/AN-ION-1-0521_Extended_Signal_Multiplexing.pdf

I added support, but the message_multiplexor_switch function makes no sense for multiplexed messages. Any suggestions how to handle that?

src/lib.rs Outdated
@@ -747,13 +767,26 @@ impl DBC {
}

/// Lookup the message multiplexor switch signal for a given message
/// ToDo This does not work for extended multiplexed messages.
Copy link
Owner

Choose a reason for hiding this comment

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

I'm wondering if it makes more sense to turn the Option into an Result type or even a Result<Option<&Signal>>, from a library user perspective I think I personally would probably prefer being aware of a possible failure by the type signature instead of running into a runtime error. What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

In my opinion Result<Option<&Signal>> would be better as it clearly distinguished between the cases: no multiplexer and a extended multiplexing.

@marcelbuesing
Copy link
Owner

Hey thanks for bearing with me. This looks really great. Also thank you for adding the reference to the documentation I was not even aware this is possible. Lets figure out the best option for this one question. I'll merge afterwards and release a new version.

@marcelbuesing marcelbuesing merged commit a267951 into marcelbuesing:dev Jun 22, 2022
@marcelbuesing
Copy link
Owner

Thanks so much again. I just tagged and released 5.0.0 containing the changes.

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.

None yet

2 participants