Skip to content

parse VAL_ attribute #14

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jul 27, 2023
Merged

parse VAL_ attribute #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6ab9cd7
Parsing VAL attributes and assigning it to the signals
Murmele Feb 16, 2023
2c3d828
Description: fix test
Murmele Feb 16, 2023
f9eb281
add more checks
Murmele Feb 16, 2023
61857a8
missing semicolon in the test leads to not reading the line correctly
Murmele Feb 16, 2023
531ef66
return false if something goes wrong
Murmele Feb 16, 2023
12ae90f
Do not use reference, because then livetime must be considered
Murmele Apr 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion include/libdbc/message.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ struct Message {
ParseSignalsStatus parseSignals(const std::vector<uint8_t>& data, std::vector<double>& values) const;

void appendSignal(const Signal& signal);
const std::vector<Signal> signals() const;
const std::vector<Signal> getSignals() const;
uint32_t id() const;
void addValueDescription(const std::string& signal_name, const std::vector<Signal::SignalValueDescriptions>&);

virtual bool operator==(const Message& rhs) const;

Expand Down
6 changes: 6 additions & 0 deletions include/libdbc/signal.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@

namespace libdbc {
struct Signal {
struct SignalValueDescriptions {
uint32_t value;
std::string description;
};

std::string name;
bool is_multiplexed;
uint32_t start_bit;
Expand All @@ -20,6 +25,7 @@ struct Signal {
double max;
std::string unit;
std::vector<std::string> receivers;
std::vector<SignalValueDescriptions> svDescriptions;

Signal() = delete;
explicit Signal(std::string name,
Expand Down
133 changes: 133 additions & 0 deletions src/dbc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,120 @@ const auto unitPattern = "\"(.*)\""; // Random string
const auto receiverPattern = "([\\w\\,]+|Vector__XXX)*";
const auto whiteSpace = "\\s";

enum VALToken { Identifier = 0, CANId, SignalName, Value, Description };

struct VALObject {
uint32_t can_id;
std::string signal_name;
std::vector<libdbc::Signal::SignalValueDescriptions> vd;
};

bool parseVal(const std::string& str, VALObject& obj) {
Copy link
Owner

Choose a reason for hiding this comment

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

This should be a member on the class of signal not external imo. External classes shouldn't know of these details just that they get the members after. Also not a huge fan of how long this function is. Perhaps split this up into some sub functions?

I am trying to follow what is going on and having a bit of a hard time. I will take another look again when I get a chance here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't think so, because this is related parsing the val object. It does the same thing like the regular expression, but in a different way because of the unknown numbers of value labels available. Therefore regex is not possible here

obj.signal_name = "";
obj.vd.clear();
auto state = Identifier;
const char* a = str.data();
libdbc::Signal::SignalValueDescriptions vd;
for (;;) {
switch (state) {
case Identifier: {
if (*a != 'V')
return false;
a++;
if (*a != 'A')
return false;
a++;
if (*a != 'L')
return false;
a++;
if (*a != '_')
return false;
a++;
if (*a != ' ')
return false;
a++; // skip whitespace
state = CANId;
break;
}
case CANId: {
std::string can_id_str;
while (*a >= '0' && *a <= '9') {
can_id_str += *a;
a++;
}
if (can_id_str.empty())
return false;
obj.can_id = std::stoul(can_id_str);
if (*a != ' ')
return false;
a++; // skip whitespace
state = SignalName;
break;
}
case SignalName: {
if ((*a >= 'a' && *a <= 'z') || (*a >= 'A' && *a <= 'Z') || *a == '_')
obj.signal_name += *a;
else
return false;
a++;
while ((*a >= 'a' && *a <= 'z') || (*a >= 'A' && *a <= 'Z') || *a == '_' || (*a >= '0' && *a <= '9')) {
obj.signal_name += *a;
a++;
}
if (*a != ' ')
return false;
a++; // skip whitespace
state = Value;
break;
}
case Value: {
std::string value_str;
while (*a >= '0' && *a <= '9') {
value_str += *a;
a++;
}
if (*a == ';') {
if (value_str.empty())
return true;
return false;
}
if (value_str.empty())
return false;

if (*a != ' ')
return false;
a++; // skip whitespace
vd.value = (uint32_t)std::stoul(value_str);
state = Description;
break;
}
case Description: {
std::string desc;
if (*a != '"')
return false;
a++;
while (*a != '"' && *a != 0) {
desc += *a;
a++;
}
if (*a == 0)
return false;
a++;
if (*a != ' ')
return false;
a++; // skip whitespace

vd.description = desc;
obj.vd.push_back(vd);

state = Value;
break;
}
}
}
return false;
}

} // anonymous namespace

namespace libdbc {
Expand Down Expand Up @@ -123,6 +237,9 @@ void DbcParser::parse_dbc_nodes(std::istream& file_stream) {
void DbcParser::parse_dbc_messages(const std::vector<std::string>& lines) {
std::smatch match;

std::vector<VALObject> sv;

VALObject obj;
Comment on lines +240 to +242
Copy link
Owner

Choose a reason for hiding this comment

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

This come back to the comment before that the message shouldn't be parsing this. Maybe the val should be found first then passed to the signal to find the ones it needs? I guess I need to think about this some and maybe the function being a member on the signal isn't the right choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea was first parsing the dbc file and then assigning the val object to the signal

for (const auto& line : lines) {
if (std::regex_search(line, match, message_re)) {
uint32_t id = std::stoul(match.str(2));
Expand All @@ -133,6 +250,7 @@ void DbcParser::parse_dbc_messages(const std::vector<std::string>& lines) {
Message msg(id, name, size, node);

messages.push_back(msg);
continue;
Copy link
Owner

Choose a reason for hiding this comment

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

why do we continue here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the regex matches a message is found and therefore it is not needed to check the others, because they will never match. So I can go directly to the next line

}

if (std::regex_search(line, match, signal_re)) {
Expand All @@ -158,6 +276,21 @@ void DbcParser::parse_dbc_messages(const std::vector<std::string>& lines) {

Signal sig(name, is_multiplexed, start_bit, size, is_bigendian, is_signed, factor, offset, min, max, unit, receivers);
messages.back().appendSignal(sig);
continue;
}

if (parseVal(line, obj)) {
sv.push_back(obj);
continue;
}
}

for (const auto& obj : sv) {
for (auto& msg : messages) {
if (msg.id() == obj.can_id) {
msg.addValueDescription(obj.signal_name, obj.vd);
Copy link
Owner

Choose a reason for hiding this comment

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

I see now that this is done through a function on the message. I see why it is done this way now I think.

break;
}
}
}
}
Expand Down
11 changes: 10 additions & 1 deletion src/message.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,23 @@ void Message::appendSignal(const Signal& signal) {
m_signals.push_back(signal);
}

const std::vector<Signal> Message::signals() const {
const std::vector<Signal> Message::getSignals() const {
return m_signals;
}

uint32_t Message::id() const {
return m_id;
}

void Message::addValueDescription(const std::string& signal_name, const std::vector<Signal::SignalValueDescriptions>& vd) {
for (auto& s : m_signals) {
if (s.name.compare(signal_name) == 0) {
s.svDescriptions = vd;
return;
}
}
}

std::ostream& operator<<(std::ostream& out, const Message& msg) {
out << "Message: {id: " << msg.id() << ", ";
out << "name: " << msg.m_name << ", ";
Expand Down
116 changes: 105 additions & 11 deletions test/test_dbc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ TEST_CASE("Testing dbc file loading", "[fileio]") {

REQUIRE(parser->get_messages() == msgs);

REQUIRE(parser->get_messages().front().signals() == msg.signals());
REQUIRE(parser->get_messages().front().getSignals() == msg.getSignals());
}
}

Expand All @@ -80,13 +80,13 @@ TEST_CASE("Testing big endian, little endian") {
parser.parse_file(filename);

REQUIRE(parser.get_messages().size() == 1);
REQUIRE(parser.get_messages().at(0).signals().size() == 2);
REQUIRE(parser.get_messages().at(0).getSignals().size() == 2);
{
const auto signal = parser.get_messages().at(0).signals().at(0);
const auto signal = parser.get_messages().at(0).getSignals().at(0);
REQUIRE(signal.is_bigendian == true);
}
{
const auto signal = parser.get_messages().at(0).signals().at(1);
const auto signal = parser.get_messages().at(0).getSignals().at(1);
REQUIRE(signal.is_bigendian == false);
}
}
Expand All @@ -104,31 +104,31 @@ TEST_CASE("Testing negative values") {
parser.parse_file(filename);

REQUIRE(parser.get_messages().size() == 1);
REQUIRE(parser.get_messages().at(0).signals().size() == 4);
REQUIRE(parser.get_messages().at(0).getSignals().size() == 4);

SECTION("Evaluating first message") {
const auto signal = parser.get_messages().at(0).signals().at(0);
const auto signal = parser.get_messages().at(0).getSignals().at(0);
REQUIRE(signal.factor == 0.1);
REQUIRE(signal.offset == 0);
REQUIRE(signal.min == -3276.8);
REQUIRE(signal.max == -3276.7);
}
SECTION("Evaluating second message") {
const auto signal = parser.get_messages().at(0).signals().at(1);
const auto signal = parser.get_messages().at(0).getSignals().at(1);
REQUIRE(signal.factor == 0.1);
REQUIRE(signal.offset == 0);
REQUIRE(signal.min == -3276.8);
REQUIRE(signal.max == -3276.7);
}
SECTION("Evaluating third message") {
const auto signal = parser.get_messages().at(0).signals().at(2);
const auto signal = parser.get_messages().at(0).getSignals().at(2);
REQUIRE(signal.factor == 10);
REQUIRE(signal.offset == 0);
REQUIRE(signal.min == -3276.8);
REQUIRE(signal.max == -3276.7);
}
SECTION("Evaluating fourth message") {
const auto signal = parser.get_messages().at(0).signals().at(3);
const auto signal = parser.get_messages().at(0).getSignals().at(3);
REQUIRE(signal.factor == 1);
REQUIRE(signal.offset == -10);
REQUIRE(signal.min == 0);
Expand All @@ -146,9 +146,103 @@ TEST_CASE("Special characters in unit") {
parser.parse_file(filename);

REQUIRE(parser.get_messages().size() == 1);
REQUIRE(parser.get_messages().at(0).signals().size() == 1);
REQUIRE(parser.get_messages().at(0).getSignals().size() == 1);
SECTION("Checking that signal with special characters as unit is parsed correctly") {
const auto signal = parser.get_messages().at(0).signals().at(0);
const auto signal = parser.get_messages().at(0).getSignals().at(0);
REQUIRE(signal.unit.compare("Km/h") == 0);
}
}

TEST_CASE("Signal Value Description") {
const auto* filename = std::tmpnam(NULL);

create_tmp_dbc_with(filename, R"(BO_ 234 MSG1: 8 Vector__XXX
SG_ State1 : 0|8@1+ (1,0) [0|200] "Km/h" DEVICE1,DEVICE2,DEVICE3
SG_ State2 : 0|8@1+ (1,0) [0|204] "" DEVICE1,DEVICE2,DEVICE3
VAL_ 234 State1 123 "Description 1" 0 "Description 2" 90903489 "Big value and special characters &$§())!" ;)");

auto parser = libdbc::DbcParser();
parser.parse_file(filename);

REQUIRE(parser.get_messages().size() == 1);
REQUIRE(parser.get_messages().at(0).getSignals().size() == 2);

REQUIRE(parser.get_messages().at(0).getSignals().at(0).svDescriptions.size() == 3);
REQUIRE(parser.get_messages().at(0).getSignals().at(1).svDescriptions.size() == 0);

const auto signal = parser.get_messages().at(0).getSignals().at(0);
REQUIRE(signal.svDescriptions.at(0).value == 123);
REQUIRE(signal.svDescriptions.at(0).description == "Description 1");
REQUIRE(signal.svDescriptions.at(1).value == 0);
REQUIRE(signal.svDescriptions.at(1).description == "Description 2");
REQUIRE(signal.svDescriptions.at(2).value == 90903489);
REQUIRE(signal.svDescriptions.at(2).description == "Big value and special characters &$§())!");
}

TEST_CASE("Signal Value Description Extended CAN id") {
/*
* It should not crash, even extended CAN id is used
*/
const auto* filename = std::tmpnam(NULL);

create_tmp_dbc_with(filename, R"(BO_ 3221225472 MSG1: 8 Vector__XXX
SG_ State1 : 0|8@1+ (1,0) [0|200] "Km/h" DEVICE1,DEVICE2,DEVICE3
SG_ State2 : 0|8@1+ (1,0) [0|204] "" DEVICE1,DEVICE2,DEVICE3
VAL_ 3221225472 State1 123 "Description 1" 0 "Description 2" 4000000000 "Big value and special characters &$§())!" ;)");

auto parser = libdbc::DbcParser();
parser.parse_file(filename);

REQUIRE(parser.get_messages().size() == 1);
REQUIRE(parser.get_messages().at(0).getSignals().size() == 2);

REQUIRE(parser.get_messages().at(0).getSignals().at(0).svDescriptions.size() == 3);
REQUIRE(parser.get_messages().at(0).getSignals().at(1).svDescriptions.size() == 0);

const auto signal = parser.get_messages().at(0).getSignals().at(0);
REQUIRE(signal.svDescriptions.at(0).value == 123);
REQUIRE(signal.svDescriptions.at(0).description == "Description 1");
REQUIRE(signal.svDescriptions.at(1).value == 0);
REQUIRE(signal.svDescriptions.at(1).description == "Description 2");
REQUIRE(signal.svDescriptions.at(2).value == 4000000000);
REQUIRE(signal.svDescriptions.at(2).description == "Big value and special characters &$§())!");
}

TEST_CASE("Signal Value Multiple VAL_") {
/*
* It should not crash, even extended CAN id is used
*/
const auto* filename = std::tmpnam(NULL);

create_tmp_dbc_with(filename, R"(BO_ 3221225472 MSG1: 8 Vector__XXX
SG_ State1 : 0|8@1+ (1,0) [0|200] "Km/h" DEVICE1,DEVICE2,DEVICE3
SG_ State2 : 0|8@1+ (1,0) [0|204] "" DEVICE1,DEVICE2,DEVICE3"
BO_ 123 MSG1: 8 Vector__XXX
SG_ State1 : 0|8@1+ (1,0) [0|200] "Km/h" DEVICE1,DEVICE2,DEVICE3
SG_ State2 : 0|8@1+ (1,0) [0|204] "" DEVICE1,DEVICE2,DEVICE3
VAL_ 3221225472 State1 123 "Description 1" 0 "Description 2" ;
VAL_ 123 State1 123 "Description 3" 0 "Description 4" ;)");

auto parser = libdbc::DbcParser();
parser.parse_file(filename);

REQUIRE(parser.get_messages().size() == 2);
REQUIRE(parser.get_messages().at(0).getSignals().size() == 2);

REQUIRE(parser.get_messages().at(0).getSignals().at(0).svDescriptions.size() == 2);
REQUIRE(parser.get_messages().at(0).getSignals().at(1).svDescriptions.size() == 0);
REQUIRE(parser.get_messages().at(1).getSignals().at(0).svDescriptions.size() == 2);
REQUIRE(parser.get_messages().at(1).getSignals().at(1).svDescriptions.size() == 0);

const auto signal = parser.get_messages().at(0).getSignals().at(0);
REQUIRE(signal.svDescriptions.at(0).value == 123);
REQUIRE(signal.svDescriptions.at(0).description == "Description 1");
REQUIRE(signal.svDescriptions.at(1).value == 0);
REQUIRE(signal.svDescriptions.at(1).description == "Description 2");

const auto signal2 = parser.get_messages().at(1).getSignals().at(0);
REQUIRE(signal2.svDescriptions.at(0).value == 123);
REQUIRE(signal2.svDescriptions.at(0).description == "Description 3");
REQUIRE(signal2.svDescriptions.at(1).value == 0);
REQUIRE(signal2.svDescriptions.at(1).description == "Description 4");
}