Skip to content

Commit

Permalink
read-api: Multiple bug fixes and make sure that all the tests still pass
Browse files Browse the repository at this point in the history
  • Loading branch information
@ThomasDebrunner
ThomasDebrunner committed Sep 25, 2023
1 parent 0343380 commit f4f49e1
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 35 deletions.
84 changes: 59 additions & 25 deletions test/ulog_parsing_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,32 @@ TEST_CASE("ULog parsing - basic test: write then read")
REQUIRE(data_container->parsingErrors().empty());
REQUIRE_FALSE(data_container->hadFatalError());

// Compare
// Check RAW API
CHECK_EQ(file_header, data_container->fileHeader());
CHECK_EQ(format1, data_container->messageFormats().at("message_name"));
CHECK_EQ(format2, data_container->messageFormats().at("other_message"));
CHECK_EQ(format1, *(data_container->messageFormats().at("message_name")));
CHECK_EQ(format2, *(data_container->messageFormats().at("other_message")));
CHECK_EQ(info, data_container->messageInfo().at("info"));
REQUIRE_EQ(data_container->logging().size(), 1);
CHECK_EQ(logging, data_container->logging()[0]);
CHECK_EQ(data_container->subscriptions().at(msg_id).data.size(), 2);
CHECK_EQ(data, data_container->subscriptions().at(msg_id).data[0]);
CHECK_EQ(data, data_container->subscriptions().at(msg_id).data[1]);
CHECK_EQ(data_container->subscriptionsByMessageId().at(msg_id)->rawSamples().size(), 2);
CHECK_EQ(data, data_container->subscriptionsByMessageId().at(msg_id)->rawSamples()[0]);
CHECK_EQ(data, data_container->subscriptionsByMessageId().at(msg_id)->rawSamples()[1]);

// Check convenience API
CHECK_EQ(format2, *(data_container->subscription("other_message")->format()));
CHECK_EQ(data_container->subscription("other_message")->size(), 2);

auto timestamp_field = data_container->subscription("other_message")->field("timestamp");
auto x_field = data_container->subscription("other_message")->field("x");

for (const auto& sample : *(data_container->subscription("other_message"))) {
// field access
CHECK_EQ(sample[timestamp_field].as<int>(), 32);
CHECK_EQ(sample[x_field].as<int>(), 49);
// direct string access
CHECK_EQ(sample["timestamp"].as<int>(), 32);
CHECK_EQ(sample["x"].as<int>(), 49);
}
}

static void readFileWriteTest(const std::filesystem::path& path,
Expand Down Expand Up @@ -227,13 +243,26 @@ TEST_CASE("ULog parsing - test corruption")

// Compare
CHECK_EQ(file_header, data_container->fileHeader());
CHECK_EQ(format2, data_container->messageFormats().at("other_message"));
CHECK_EQ(format2, *(data_container->messageFormats().at("other_message")));
// Here we inserted zero bytes, but the next messages should not be dropped
REQUIRE_EQ(data_container->logging().size(), 1);
CHECK_EQ(logging, data_container->logging()[0]);
CHECK_EQ(data_container->subscriptions().at(msg_id).data.size(), 2);
CHECK_EQ(data, data_container->subscriptions().at(msg_id).data[0]);
CHECK_EQ(data, data_container->subscriptions().at(msg_id).data[1]);
CHECK_EQ(data_container->subscriptionsByMessageId().at(msg_id)->rawSamples().size(), 2);
CHECK_EQ(data, data_container->subscriptionsByMessageId().at(msg_id)->rawSamples()[0]);
CHECK_EQ(data, data_container->subscriptionsByMessageId().at(msg_id)->rawSamples()[1]);

auto timestamp_field = data_container->subscription("other_message")->field("timestamp");
auto x_field = data_container->subscription("other_message")->field("x");

for (const auto& sample : *(data_container->subscription("other_message"))) {
// field access
CHECK_EQ(sample[timestamp_field].as<int>(), 32);
CHECK_EQ(sample[x_field].as<int>(), 49);
// direct string access
CHECK_EQ(sample["timestamp"].as<int>(), 32);
CHECK_EQ(sample["x"].as<int>(), 49);
}

}

struct MyData {
Expand Down Expand Up @@ -334,25 +363,30 @@ TEST_CASE("ULog parsing - simple writer")
REQUIRE_FALSE(data_container->hadFatalError());

// Compare
CHECK_EQ(sys_name,
std::get<std::string>(data_container->messageInfo().at("sys_name").value().data()));
CHECK_EQ(sys_name, data_container->messageInfo().at("sys_name").value().as<std::string>());
REQUIRE_EQ(data_container->logging().size(), 1);
CHECK_EQ(text_message, data_container->logging()[0].message());
CHECK_EQ(param_a,
std::get<float>(data_container->initialParameters().at("PARAM_A").value().data()));
CHECK_EQ(param_b,
std::get<int32_t>(data_container->initialParameters().at("PARAM_B").value().data()));
CHECK_EQ(param_a, data_container->initialParameters().at("PARAM_A").value().as<float>());
CHECK_EQ(param_b, (data_container->initialParameters().at("PARAM_B").value().as<int32_t>()));

CHECK_EQ(MyData::messageName(),
data_container->messageFormats().at(MyData::messageName()).name());
REQUIRE_EQ(data_container->subscriptions().size(), 1);
const auto& data_array = data_container->subscriptions().begin()->second.data;
REQUIRE_EQ(data_array.size(), written_data_messages.size());
for (unsigned i = 0; i < data_array.size(); ++i) {
MyData data{};
REQUIRE_GE(sizeof(data), data_array[i].data().size());
memcpy(&data, data_array[i].data().data(), data_array[i].data().size());
CHECK_EQ(written_data_messages[i], data);
data_container->messageFormats().at(MyData::messageName())->name());
REQUIRE_EQ(data_container->subscriptionNames().size(), 1);

const auto subscription = data_container->subscription(MyData::messageName());
REQUIRE_EQ(subscription->size(), written_data_messages.size());
for (size_t i=0; i<subscription->size(); ++i) {
const auto &sample = (*subscription)[i];
const auto &gt = written_data_messages[i];
MyData memcopied_sample{};
REQUIRE_GE(sizeof(MyData), sample.rawData().size());
// check raw byte equality
memcpy(&memcopied_sample, sample.rawData().data(), sample.rawData().size());
CHECK_EQ(gt, memcopied_sample);
// check API access field equality
CHECK_EQ(gt.timestamp, sample["timestamp"].as<uint64_t>());
CHECK_EQ(gt.cpuload, sample["cpuload"].as<float>());
CHECK_EQ(gt.counter, sample["counter"].as<int8_t>());
}
}

Expand Down
46 changes: 41 additions & 5 deletions ulog_cpp/data_container.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,47 @@ void DataContainer::error(const std::string& msg, bool is_recoverable)
}
void DataContainer::headerComplete()
{
// try to resolve all message formats
// try to resolve all fields in message formats
for (auto& it : _message_formats) {
auto& message_format = it.second;
message_format->resolveDefinition(_message_formats);
}

// try to resolve all fields for all message infos
for (auto& it : _message_info) {
auto& message_info = it.second;
message_info.field().resolveDefinition(_message_formats, 0);
}

// try to resolve all fields for params
for (auto &it : _default_parameters) {
auto &parameter = it.second;
parameter.field().resolveDefinition(_message_formats, 0);
}

for (auto &it : _initial_parameters) {
auto &parameter = it.second;
parameter.field().resolveDefinition(_message_formats, 0);
}

for (auto &parameter : _changed_parameters) {
parameter.field().resolveDefinition(_message_formats, 0);
}

_header_complete = true;
}
void DataContainer::fileHeader(const FileHeader& header)
{
_file_header = header;
}
void DataContainer::messageInfo(const MessageInfo& message_info)
void DataContainer::messageInfo(const MessageInfo& message_info_arg)
{
// create mutable copy
MessageInfo message_info = message_info_arg;
if (_header_complete) {
// if header is complete, we can resolve definition here
message_info.field().resolveDefinition(_message_formats, 0);
}
if (_header_complete && _storage_config == StorageConfig::Header) {
return;
}
Expand All @@ -57,19 +85,27 @@ void DataContainer::messageFormat(const MessageFormat& message_format)
}
_message_formats.insert({message_format.name(), std::make_shared<MessageFormat>(message_format)});
}
void DataContainer::parameter(const Parameter& parameter)
void DataContainer::parameter(const Parameter& parameter_arg)
{
if (_header_complete && _storage_config == StorageConfig::Header) {
return;
}
if (_header_complete) {
auto parameter = parameter_arg;
// if header is complete, we can resolve definition here
parameter.field().resolveDefinition(_message_formats, 0);
_changed_parameters.push_back(parameter);
} else {
_initial_parameters.insert({parameter.field().name(), parameter});
_initial_parameters.insert({parameter_arg.field().name(), parameter_arg});
}
}
void DataContainer::parameterDefault(const ParameterDefault& parameter_default)
void DataContainer::parameterDefault(const ParameterDefault& parameter_default_arg)
{
ParameterDefault parameter_default = parameter_default_arg;
if (_header_complete) {
// if header is already complete, we can resolve definition here
parameter_default.field().resolveDefinition(_message_formats, 0);
}
_default_parameters.insert({parameter_default.field().name(), parameter_default});
}
void DataContainer::addLoggedMessage(const AddLoggedMessage& add_logged_message)
Expand Down
4 changes: 2 additions & 2 deletions ulog_cpp/messages.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,8 @@ void MessageFormat::serialize(const DataWriteCB& writer) const
{
std::string format_str = _name + ':';

for (const auto& field : _fields) {
format_str += field.second->encode() + ';';
for (const auto& field : _fields_ordered) {
format_str += field->encode() + ';';
}

ulog_message_format_s format;
Expand Down
20 changes: 17 additions & 3 deletions ulog_cpp/messages.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,10 @@ class Field {
auto it = kBasicTypes.find(type_str);
if (it != kBasicTypes.end()) {
_type = it->second;
} else {
// if not a basic type, set it to recursive
_type = TypeAttributes(type_str, BasicType::NESTED, 0);
}
// if not a basic type, set it to recursive
_type = TypeAttributes(type_str, BasicType::NESTED, 0);
}

std::string encode() const;
Expand Down Expand Up @@ -255,6 +256,7 @@ class MessageInfo {
MessageInfo(const std::string& key, float value);

const Field& field() const { return _field; }
Field& field() { return _field; }
std::vector<uint8_t>& valueRaw() { return _value; }
const std::vector<uint8_t>& valueRaw() const { return _value; }
Value value() const { return Value(_field, _value); }
Expand Down Expand Up @@ -289,7 +291,18 @@ class MessageFormat {
void serialize(const DataWriteCB& writer) const;
bool operator==(const MessageFormat& format) const
{
return _name == format._name && _fields == format._fields;
if (_name != format._name) {
return false;
}
if (_fields.size() != format._fields.size()) {
return false;
}
for (size_t i=0; i<_fields_ordered.size(); i++) {
if (!(*_fields_ordered[i] == *format._fields_ordered[i])) {
return false;
}
}
return true;
}

void resolveDefinition(const std::map<std::string, std::shared_ptr<MessageFormat>>& existing_formats) const;
Expand Down Expand Up @@ -327,6 +340,7 @@ class ParameterDefault {
ulog_parameter_default_type_t default_types);

const Field& field() const { return _field; }
Field& field() { return _field; }
const std::vector<uint8_t>& valueRaw() const { return _value; }
Value value() const { return Value(_field, _value); }

Expand Down
6 changes: 6 additions & 0 deletions ulog_cpp/subscription.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ class TypedDataView {
std::string name() const { return _message_format_ref.name(); }

Value at(const Field &field) const {
if (!field.definitionResolved()) {
throw ParsingException("Field definition not resolved");
}
return Value(field, _data_ref.data());
}

Expand All @@ -45,6 +48,9 @@ class TypedDataView {
return at(field_name);
}

const MessageFormat& format() const { return _message_format_ref; }
const std::vector<uint8_t> &rawData() const { return _data_ref.data(); }

private:
const Data& _data_ref;
const MessageFormat& _message_format_ref;
Expand Down

0 comments on commit f4f49e1

Please sign in to comment.