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

Runtime error when reading INT Signal as an Integer - Interpreted as Double #130

Open
linkret opened this issue Jan 26, 2023 · 5 comments · May be fixed by #131
Open

Runtime error when reading INT Signal as an Integer - Interpreted as Double #130

linkret opened this issue Jan 26, 2023 · 5 comments · May be fixed by #131
Labels
bug Something isn't working

Comments

@linkret
Copy link

linkret commented Jan 26, 2023

My .dbc file says this:

BA_DEF_ SG_ "SignalID" INT 0 100000 ;
BA_ "SignalID" SG_  786 BMSmaxPackTemperature 237;

In code, I get the ISignal, and ask for its -> AttributeValues(), iterating over them. If I try to do std::get<0>(a.Value()) (fetch the Attribute value as an INTEGER, because the value_t the .Value() method returns is an std::variant<int64_t, double, std::string>), my program crashes. Turns out that the attribute was parsed as a FLOAT, and the program only works if I do std::get<1>(a.Value()), and try to read it as a real number.

@xR3b0rn
Copy link
Owner

xR3b0rn commented Jan 26, 2023

Can you provide a more precise example? Show me your source code or something.

@linkret
Copy link
Author

linkret commented Jan 26, 2023

The entire code is too long, but this is the main part. The ISignal* was gotten from an IMessage->Signals().

    const dbcppp::ISignal* _sig_def;
    ...
    inline int64_t signal_id() const {
        for (const auto& a : _sig_def->AttributeValues()) {
            if (a.Name() == "SignalID") return std::get<1>(a.Value()); // 1 = FLOAT instead of 0 = INT
        }
        return -1;
    }

Runtime error with std::get<0>, correct result with std::get<1> (double). _sig_def->ExtendedValueType() returns 1 for FLOAT, when the .dbc file says the Attribute is an INT.
For strings, std::get<2> works as expected.

@xR3b0rn
Copy link
Owner

xR3b0rn commented Jan 26, 2023

I checked the grammar parser, and it seems like this is due to an error in the grammar (here). The marked line says:

static const auto attribute_value_def = double_ | signed_int | quoted_string;

This line will make the grammar parser first try to parse the value as an double, and only if it fails, continue with trying to parse the value as int. However, since every int is a double, the parser always will parse int as double. Swaping double_ and signed_int should fix your specific problem. However, then a new appears: When the BA_DEF_ is defining the value as floating point but the parser is able to parse the BA_ value as int, the opposite of your error happens, what of course is an error as well. But since the "new error" is an error which must semantically be catched later (e.g. when parsing the AST), swaping is the right thing to do here.

Can you fix this in your local dbcppp version and confirm my theory?

@linkret
Copy link
Author

linkret commented Jan 26, 2023

Yes, I can confirm this fixes it. Thanks so much!

xR3b0rn added a commit that referenced this issue Jan 26, 2023
@xR3b0rn xR3b0rn linked a pull request Jan 26, 2023 that will close this issue
@xR3b0rn
Copy link
Owner

xR3b0rn commented Jan 26, 2023

The tests are failing, you should use the dirty fix with care.

@xR3b0rn xR3b0rn added the bug Something isn't working label Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants