EFeru / DbcParser

.NET CAN bus dbc file parser
MIT License
72 stars 26 forks source link

#72 #69 #37 #14 #35 Redfine all Model classes to be readonly for public access. Use FinishUp method to calc useful properties and support for extended Multiplexing. #73

Closed Uight closed 2 weeks ago

Uight commented 3 weeks ago

All classes belonging to the model namespace are now readonly for public access. The immutable stuff was therefor removed. Internally all values can be written as before. Every class that profits from it now has a FinishUp method to calculate some usefull properties. Also the signals and the messages are now a dictionary which should resolve #14.

Some changes are breaking changes!!! (e.g dbc is now part of model namespace)

In the finish up methods i also update the MessageID for all signals in a message after applying the extendedId which should fix #72. Also the IsMultiplexed value is also calculates once in this FinishUp method to resolve #37

The targetFramework was set to .net462, netstandard2.0 and .net8.0 to resolve #69

Also there are some other properties in the signal class like HasScaling which would help if extending the packling/unpacking functionalities for a next step after #67

Edit: Now should also resolve #35

Uight commented 3 weeks ago

im now working on a follow up to this. i enabled nullability for the dbcParserLib which makes it easier to see which properties could actually be null. I needed this for ExtendedMultiplexing as i adjusted the MultiplexingInfo to contain either the simple info or the complex info or none if the signal is not multiplexed. im currently trying to figure out a way to make access to custom properties better. (all values are nullable and one is set atm). When i figure that out and this PR should be merged allready ill create a new PR or ill update this one.

Uight commented 3 weeks ago

With the last commit i added support for Extended Multiplexing in attempt to resolve #35 because im using nullables there i changes the project to define that but then i realized that tons of stuff in the custom properties is nullable. so i changed the properties to use interfaces instead of 5 seperate fields which i think is nicer but now i think i should probably use a interface for the multiplexing too.

@EFeru you might wanna check the new extended multiplexing support. i suppose all information is there now but im not so sure about the structure. Also please check out the design for the model classes. I think the way it is now is atleast better as before but could probably still be improved im open to suggestens.

Im also not to sure about these three functions: FillNodesNotSetCustomPropertyWithDefault FillMessagesNotSetCustomPropertyWithDefault FillSignalsNotSetCustomPropertyWithDefault

I decided to set the defautl only if a default exists which i think is not required and only if the value has not been set yet. However this could be changes to accomodate #70 i think at least.

Uight commented 3 weeks ago

Im now considering this more or less done. If merged i would suggest doing this as a 2.0 Version as many things changed.

As for the extended multiplexing support i cleaned that up but as for now the order is pretty much reversed. You can find the multiplexor for any multiplexed signal. But if you want the signals a multiplexor applies to you would need to search yourself. This is because the way extended multiplexing is defined. However you could add the multiplexed signals to a multiplexor after all the multiplexing was finalized.

Adhara3 commented 3 weeks ago

Hi,

thanks for the effort. I would nevertheless try to split this one into multiple PRs. Changing the API is very different than adding full multiplexed support. At least the former requires some alignement on design decisions. It's a bit tricky in these issues comments, but if we have no other way to communicate, let's open a new issue talking about design targets and API.

Cheers A

Uight commented 3 weeks ago

@Adhara3 im not sure if splitting this up is possible in a good way. Sure i could remove the framework stuff then probably the check would also go thorugh as UnitTest is running to. The problem with the multiplexed stuff is that i based that on teh methods for finishing up stuff added with the rest of the api change. If you want to open a seperate issue for the api stuff i would be happy to discuss.