Skip to content

[20274] Validate the YAML configuration file on parsing#113

Open
tempate wants to merge 8 commits intomainfrom
feature/yaml_validator
Open

[20274] Validate the YAML configuration file on parsing#113
tempate wants to merge 8 commits intomainfrom
feature/yaml_validator

Conversation

@tempate
Copy link
Contributor

@tempate tempate commented Jan 22, 2024

In this version, the DDS Recorder and the DDS Replayer throw a warning when a YAML tag is ignored to prevent typos, misplacements, and wrong configurations.

Merge after:

tempate added 7 commits September 19, 2024 08:50
Signed-off-by: tempate <danieldiaz@eprosima.com>

Validate YAML tags on parsing

Signed-off-by: tempate <danieldiaz@eprosima.com>

Validate new YAML options

Signed-off-by: tempate <danieldiaz@eprosima.com>

Make sets static

Signed-off-by: tempate <danieldiaz@eprosima.com>
Signed-off-by: tempate <danieldiaz@eprosima.com>
Signed-off-by: tempate <danieldiaz@eprosima.com>
Signed-off-by: tempate <danieldiaz@eprosima.com>
Signed-off-by: tempate <danieldiaz@eprosima.com>
Signed-off-by: tempate <danieldiaz@eprosima.com>
Signed-off-by: tempate <danieldiaz@eprosima.com>
}
}

void RecorderConfiguration::load_output_configuration_(
Copy link
Contributor

Choose a reason for hiding this comment

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

You are not using this method, delete it or complete it.

int exit(ProcessReturnCode code)
{
// Delete the consumers before closing
eprosima::utils::Log::ClearConsumers();
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this flush as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently it is called in ClearConsumers , so I guess not.

Comment on lines +204 to +209
// NOTE:
// It will not filter any log, so Fast DDS logs will be visible unless Fast DDS is compiled
// in non debug or with LOG_NO_INFO=ON.
// This is the easiest way to allow to see Warnings and Errors from Fast DDS.
// Change it when Log Module is independent and with more extensive API.
// utils::Log::SetCategoryFilter(std::regex("(DDSRECORDER)"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this note make sense anymore?


YamlValidator::validate_tags(yml, tags);

mcap::McapWriterOptions mcap_writer_options{"ros2"};
Copy link
Contributor

Choose a reason for hiding this comment

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

Inner tags are not validated right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe they are


/////
// Get optional specs configuration
// WARNING: Parse builtin topics (dds tag) AFTER specs, as some topic-specific default values are set there
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this warning is important, I would update it not delete it.


This release includes the following **Recording features**:

* :ref:`Resource Limits <recorder_specs_logging>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete.

Signed-off-by: Lucia Echevarria <luciaechevarria@eprosima.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants