Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

#23 Parsing failures management #42

Merged
merged 6 commits into from
Sep 11, 2023

Conversation

Whitehouse112
Copy link
Contributor

  • Added IParseFailureObserver interface that defines parsing errors handling methods. Methods deal with both syntax errors and duplicated/not found key/name errors for nodes, messages, signals and so on.

  • Provided two implemetations for the interface:
    1 - SilentFailureObserver: does nothing, like today. It's the default observer
    2 - SimpleFailureObserver: save each error in a list of string

  • Added several tests for each parsing failure error

  • Update Demo application to print errors, if any, using the SimpleFailureObserver class.

Added observer to listen for parsing failures. Code clean up.
Added parsing failures management for each line parser.
Created silent and simple failure observer classes
Added parsing failure tests. Removed old and no more valid tests
…e character is used as separator. Updated and added some test

EnumCustomProperty parsing fails if more than one white-space chracter is used as separator. Updated and added some test, code clean up.
@EFeru
Copy link
Owner

EFeru commented Aug 29, 2023

Hi @Whitehouse112 , thanks for the effor in this PR. I am not sure if it is nomal, however if I run the Demo, I get the follwing warnings/failures:
image

@Whitehouse112
Copy link
Contributor Author

Hi @EFeru,

  • [VAL_] Value table syntax error: the syntax specification implemented in this library doesn't support a signed integer as a valid value description for the VAL_ keyword. Syntax is VAL_ var_name unsigned_integer char_string. The reported lines contain a negative number, from which the syntax error
  • Unknown syntax error: this is something I need to change, it regards the NS_ definition at the beginning of the file. My though was to support only symbols actually implemented by the library itself but there is no reason to display this failures after all. I will fix asap.

Regards

Now every NS definition at the top of a file is ignored
@EFeru
Copy link
Owner

EFeru commented Sep 1, 2023

"Unknown syntax" is fixed. The following remained:
image

If I look in the dbc file indeed I see negative values for VAL_, is this something not supported by this library or the dbc file is not according to dbc Vector specs?
image

@Whitehouse112
Copy link
Contributor Author

Whitehouse112 commented Sep 1, 2023

  • It's something not supported by this library? Yes, It's not supported
  • It's not compliant to the dbc Vector spec? I don't know: the only documentation available on the internet is the one provided also in this library and a signed integer is not a valid value for VAL_ tag.

image

So this library supports only unsigned integer since the only documentation available states that. This document is from 2010, I don't know if it has been updated, I looked for a recent version without success.

@Adhara3
Copy link
Collaborator

Adhara3 commented Sep 4, 2023

Hi,

It's very important that we find a reference and we stick with it. The document linked by @Whitehouse112 is by far the most complete and detailed resource we could find on the web.
Usually we also look for other software behaviour, we have the feeling that the specs are a bit sloppy so software got quite tolerant on syntax over time.

How is CANdb++ behaving with those lines?

My 2 cents
A

@EFeru
Copy link
Owner

EFeru commented Sep 6, 2023

CANdb++ just adds the value description without complains.
image

Edit: Just for completness, for the tesla dbc file, CANdb++ gives these results when running the consistency checks:
image

@EFeru EFeru merged commit 6fcf801 into EFeru:main Sep 11, 2023
@EFeru EFeru mentioned this pull request Sep 14, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants