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

Using Generics for messages #1239

Merged
merged 30 commits into from
May 3, 2024

Conversation

InvincibleRMC
Copy link
Contributor

First Draft.

@InvincibleRMC InvincibleRMC marked this pull request as ready for review March 5, 2024 03:11
@fujitatomoya fujitatomoya self-assigned this Mar 14, 2024
@InvincibleRMC InvincibleRMC marked this pull request as draft March 22, 2024 00:15
@InvincibleRMC
Copy link
Contributor Author

Noticed bug in protocol definitions working on a fix.

@InvincibleRMC
Copy link
Contributor Author

Until this gets merged ros2/rosidl_python#205 the type of all messages/services/actions have to be inferred as Any due to PEP 561. Because messages are all inferred to be Any It won't currently alert user about putting in the wrong message type. But I tested it by adding a py.typed file into std_msgs it correctly warns the user about different messages.

InvincibleRMC and others added 22 commits March 24, 2024 12:45
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <36806982+InvincibleRMC@users.noreply.github.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
* Testing out Enum wrapper for ClockType

* convert to rcl_clock_type_t

* Update create_time_point

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
* Add types to exception

* Add type checking guard

* Fix NotInitializedException

* Add missing defualt

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: SnIcK <ido.samuelson@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
* Add types to logging_service.py (ros2#1227)

* add types to logging_service

* Add types to duration.py

* Add newlines for class definintions

* update type alias name

* Update to use Protocols

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
* Add types to validate files

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* remove type annotations from docstrings

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* removed other type annotated docstrings

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

---------

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
* Add types to TypeHash and moved away from __slots__ usage

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* remove docstring types

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

---------

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
* Start typing time.py

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Testing out Enum wrapper for ClockType

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* convert to rcl_clock_type_t

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Undo Change to time_point.cpp

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Update create_time_point

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Lint fixes

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Add debug message

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Remove test file

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Try extending the type assert

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Add types to logging_service.py (ros2#1227)

* add types to logging_service

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Add types to duration.py

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Add newlines for class definintions

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* update type alias name

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Remove newline

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Merge?

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Fix failed merge

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Update to use Protocols

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Fix import error

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Add types to time.py

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Linty

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

---------

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
…s2#1232)" (ros2#1243)

This reverts commit b06baef.

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
* Add types to TypeHash and add test cases

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC InvincibleRMC marked this pull request as ready for review March 24, 2024 16:46
@InvincibleRMC InvincibleRMC changed the title Using Generics for messages and services Using Generics for messages Mar 24, 2024
@InvincibleRMC
Copy link
Contributor Author

Limited scope to Generics for publishers and subscribers only because couldn't get generics of services and clients working the way I wanted.

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@sloretz
Copy link
Contributor

sloretz commented Apr 12, 2024

Mini CI just to see what happens (repos file build: --packages-up-to rclpy test: --packages-select rclpy)

Linux: Build Status

Signed-off-by: Michael Carlstrom <36806982+InvincibleRMC@users.noreply.github.com>
Copy link
Contributor

@sloretz sloretz left a 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!

…RMC/rclpy into InvincibleRMC-generic-messages-and-services

Signed-off-by: Shane Loretz <sloretz@intrinsic.ai>
@sloretz sloretz force-pushed the generic-messages-and-services branch from 010cf33 to 468137a Compare May 3, 2024 18:13
@sloretz
Copy link
Contributor

sloretz commented May 3, 2024

CI (repos file build: --packages-up-to rclpy test: --packages-select rclpy)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@sloretz sloretz merged commit 99f1ce9 into ros2:rolling May 3, 2024
3 checks passed
@sloretz
Copy link
Contributor

sloretz commented May 3, 2024

Thank you for the PR!

@InvincibleRMC
Copy link
Contributor Author

Thank you for the PR!
@sloretz Would it be possible to backport this and future typing changes into Jazzy as well?

@sloretz
Copy link
Contributor

sloretz commented May 3, 2024

Seems safe to back port to me

@Mergifyio backport jazzy

@clalancette
Copy link
Contributor

Seems safe to back port to me

While I'm OK with backporting this, I'm going to suggest we hold off on doing it until after Jazzy is released. That way we can be certain that only critical bugfixes are going in at this time.

@InvincibleRMC
Copy link
Contributor Author

Seems safe to back port to me

While I'm OK with backporting this, I'm going to suggest we hold off on doing it until after Jazzy is released. That way we can be certain that only critical bugfixes are going in at this time.

Seems very reasonable.

@InvincibleRMC InvincibleRMC deleted the generic-messages-and-services branch May 18, 2024 19:55
@InvincibleRMC InvincibleRMC restored the generic-messages-and-services branch May 18, 2024 19:55
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.

None yet

5 participants