-
Notifications
You must be signed in to change notification settings - Fork 793
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
[21184] Refactor XML Parser to return DynamicTypeBuilder instead of DynamicType #4970
Conversation
b7b621c
to
97ec783
Compare
b42806b
to
9b181e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good refactor ! Leaving some suggestions.
In addition, couple of considerations:
- Please, initialize
type_discovered_(false)
in theSubscriberApp.cpp
, stop_(false) Xtypes
example refactor was merged). - Also, I am getting a
AddressSanitizer:DEADLYSIGNAL
if I start the publisher with--xml-type
(compiled with ASAN), we may need to investigate it further.
f1064ba
to
6cbc19c
Compare
I have tried to reproduce the error, and it appears when the xml file is not uploaded through environment variable. Can you check if that is the case? |
6cbc19c
to
5602945
Compare
c727706
to
418d2fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving unresolved just a couple of conversations above that I think were not included
Suggestions applied!
|
82ac266
to
8294e3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with green CI
…etDynamicTypeByName to get a DynamicTypeBuilder Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
…m xml Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
…le works Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
…nullptr Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
…TypeBuilder Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
8294e3e
to
b4b5edf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
This PR is a small refactor of the methods to get dynamic types from XML in order to return a
DynamicTypeBuilder
object instead of aDynamicType
one. It also add a CLI flag to xtypes example to load types and profiles from an xml files.Contributor Checklist
versions.md
file (if applicable).Related documentation PR: [21184] Adjust type in get_dynamic_type_builder_from_xml_by_name to return DynamicTypeBuilder Fast-DDS-docs#818 -->
Reviewer Checklist