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

Allow fully qualified name in TypeDescriptor [11063] #1831

Merged
merged 4 commits into from
Jun 7, 2021

Conversation

YangboLong
Copy link
Contributor

According to the DDS-XTypes spec 7.5.2.4.10, the name of TypeDescriptor
should be the fully qualified name, which is the concatenation of
module names with the name of a type inside of those modules.

This change will allow an object of DynamicTypeBuilder to set a type
name that contains colon in it. Sometimes it's necessary to use a fully
qualified name in dynamic types to communicate to some existing
ros2 topic directly.

Related issue: https://gitmemory.com/issue/eProsima/Fast-RTPS-Gen/21/603785610

@YangboLong
Copy link
Contributor Author

YangboLong commented Apr 2, 2021

I just noticed that this PR #1345 attempted to solve the same problem. Would you let me know what's wrong with my PR?

Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

The logic looks good to me.

Please add some unit tests here

@MiguelCompany MiguelCompany changed the title Allow fully qualified name in TypeDescriptor Allow fully qualified name in TypeDescriptor [11063] Apr 5, 2021
@MiguelCompany
Copy link
Member

Would you let me know what's wrong with my PR?

It is just that we didn't have time to do the review. Could you please fix DCO and linters?

According to the DDS-XTypes spec 7.5.2.4.10, the name of TypeDescriptor
should be the fully qualified name, which is the concatenation of
module names with the name of a type inside of those modules.

This change will allow an object of DynamicTypeBuilder to set a type
name that contains colon in it.

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>
Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>
Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>
Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>
@YangboLong
Copy link
Contributor Author

I've added a few test cases and fixed the DCO and linters. Would you please review it again?

@MiguelCompany
Copy link
Member

@richiprosima Please test this

Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM

@MiguelCompany
Copy link
Member

@Mergifyio backport 2.2.x 2.1.x 2.0.x

mergify bot pushed a commit that referenced this pull request Jun 9, 2021
* Allow fully qualified name in TypeDescriptor

According to the DDS-XTypes spec 7.5.2.4.10, the name of TypeDescriptor
should be the fully qualified name, which is the concatenation of
module names with the name of a type inside of those modules.

This change will allow an object of DynamicTypeBuilder to set a type
name that contains colon in it.

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>

* Try to fix linters

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>

* Add some unit tests for type descriptor names

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>

* Fix linters

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>
(cherry picked from commit 61267e3)
mergify bot pushed a commit that referenced this pull request Jun 9, 2021
* Allow fully qualified name in TypeDescriptor

According to the DDS-XTypes spec 7.5.2.4.10, the name of TypeDescriptor
should be the fully qualified name, which is the concatenation of
module names with the name of a type inside of those modules.

This change will allow an object of DynamicTypeBuilder to set a type
name that contains colon in it.

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>

* Try to fix linters

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>

* Add some unit tests for type descriptor names

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>

* Fix linters

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>
(cherry picked from commit 61267e3)
mergify bot pushed a commit that referenced this pull request Jun 9, 2021
* Allow fully qualified name in TypeDescriptor

According to the DDS-XTypes spec 7.5.2.4.10, the name of TypeDescriptor
should be the fully qualified name, which is the concatenation of
module names with the name of a type inside of those modules.

This change will allow an object of DynamicTypeBuilder to set a type
name that contains colon in it.

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>

* Try to fix linters

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>

* Add some unit tests for type descriptor names

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>

* Fix linters

Signed-off-by: Yangbo Long <yangbo.long.mav@gmail.com>
(cherry picked from commit 61267e3)
@mergify
Copy link
Contributor

mergify bot commented Jun 9, 2021

Command backport 2.2.x 2.1.x 2.0.x: success

Backports have been created

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.

2 participants