#15 User defined attributes support #29
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added support for BA_DEF_DEF_ tag - Added support for BA_DEF_ tag - Added support for BA_ tag
Added test for: - Message custom property - Signal custom property - Node custom property Changed regex parsing string
- HexCustomProperty class merged into IntCustomProperty class - PropertyLineParser classe splitted into different files - Different classes for property definition and property assignment - Improved regex for enum parsing - Added some tests
Added custom property tests
Adhara3
requested changes
Feb 23, 2023
There was a problem hiding this comment.
CustomProperty.CustomPropertyDefinitionshould be public, should't it? How the user would know the property data type otherwise?- I would not hide the
HEXdata type. To me it should stay in the enum and map to it's own property both in definition and value. The mapped type should be an integer type (probablyunsigned integer) - Same applies to
EnumValue, there should be apublic CustomPropertyValue<string> EnumCustomProperty { get; set; }. This is not mandatory but it would make things clearer from an API/user pov EnumCustomProperty.DefinitionI would probably call itEnumerationorValuesinstead and the type should beISet<string>to make sure they are unique non duplicate values- Property
CustomPropertiesforSignal,NodeandMessageshould be a
IReadonlyDictionary<string, CustomProperty>or eventually a custom type with a simple interface (e.g.bool ContainsProperty(string propertyName),bool TryGetProperty(string name, out CustomProperty property),int Count {get;}. This is similar to what we will do forDbcclass collections) - Make sure writing some tests that the regex also accept floating point numbers in scientific notation (i.e not only 0.01 but also 1e-2)
- Split classes into separate files
… some tests. Code refactoring - Node, Message and Signal classes are now immutable - Created new mutable classes for Node, Message and Signal - CustomProperties (BA_) regex now supports scientific notation for double - Added tests for regex scientific notation support - Added Enum and Hex properties in CustomProperty and CustomPropertyDefinition classes - Updated tests to support new classes - Code refactoring
- Editable classes (Message, Node, Signal) are now internal - LineParser classes are now internal - ILineParser and IDbcBuilder interfaces are now internal
- Final immutable objects are now immutable classes (eg: Node -> ImmutableNode) - Mutable classes are now final objects (eg: MutableNode -> Node) - Immutable classes are not used anymore (ready to be implemented in the future)
Adhara3
approved these changes
Mar 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.