Skip to content

Conversation

@jacobperron
Copy link
Contributor

Contains significant changes/additions related to Actions, QoS, ROS time, and graph APIs. This brings in all changes we've developed on our fork that are compatible with Galactic: https://github.com/osrf/ros2_java/tree/galactic-devel

For posterity, each commit references the pull request to the OSRF fork where review and any discussion occurred.

ivanpauno and others added 30 commits May 17, 2022 15:16
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Add macros to handle java exceptions
Add macros to throw a java exception from an rcl error
Always use `base_message` instead of `message` in docblocks

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Improve QoSProfile constructors
* Add documentation to QoSProfile class
* Add fromRCL method to QoSProfile
* Use fromRCL to create the built-in qos profiles.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Add NameAndTypes class
* Add getTopicNamesAndTypes method to Node

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
** Add EndpointInfo class.
* Add getPublishersInfo method to Node.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Add NodeNameInfo class
* Implement getNodeNames

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
…osrf#38)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
In these cases, we are likely leaking memory.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Start revamping Time and Clock classes.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>

* Fixes from review.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>

* Add in logging to the TimeSource class.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Document TimeSource class

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Address feedback

* Empty -> Default

* Add more detail to class docblock

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Support wall-time and ROS-time timers

Add a new method to Node for creating ROS timers.

Refactor timer implementation to account for the two different types of timers.
Deprecate WallTimer interface and WallTimerImpl and replace with unified Timer interface and TimerImpl.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Log error if failed to dispose timer

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Useful for getting the current time from the Clock instance.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
…rf#35)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
This fixes two issues:

1. The default constructor for ParameterAndDescriptor initializes it's members to null.
  This can cause a segfault when trying to access a parameters descriptor if undeclared
  parameters are allowed and it has been set without a descriptor
  (e.g. using setParameters()).

  This change defines the constructor for ParameterAndDescriptor so that we don't hit
  any null values.

2. Parameters that are not declared and set with setParameters() do not have their
  descriptors set to the correct value.
  This change makes sure that the descriptor is initialized with the values of the
   parameter being set.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
The workAvailable flag was never being set to true.
If I understand the original intention, I think that the flag should be set as long as there are things being executed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Now we're actually running the tests.

This also fixes a bug where we were comparing the wrong objects in one of the tests.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* change logic(list -> array) in msg.java.em

* change logic(list -> array) in msg.cpp.em

* Support getting and setting as Lists in addition to arrays

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update rcljava to work with new methods for list types

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update tests to work with new methods for list types

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Fixes to template files

* Avoid nullptr access when converting from Java arrays to C arrays
* Make sure Java arrays are initialized

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* A note about performance to *AsList docblocks

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

Co-authored-by: pluris <zighart8456@naver.com>
* Add interfaces for action goal, result, and feedback

Implementing these interfaces in the code generation template makes it easier to pass around these types in a generic way.
Note, the 'final' modifier had to be removed from generated message types in order to extend goal, result, and feedback types in action definitions.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add new definitions for action goal response and request

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add getter for UUID to SendGoalRequest

Also make inner classes static.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add getStamp method to GoalResponseDefinition

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Partially revert "Add interfaces for action goal, result, and feedback"

Partially revert commit dd04614.

I don't think we need to aliases for the message types, but I'll add them back if they turn out to be useful.

* Parameterize goal request and response interfaces on action type

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Fix getGoalUuid implementation

It should return a List, since it is hashable.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Add interfaces for action goal, result, and feedback

Implementing these interfaces in the code generation template makes it easier to pass around these types in a generic way.
Note, the 'final' modifier had to be removed from generated message types in order to extend goal, result, and feedback types in action definitions.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add new definitions for action goal response and request

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add getter for UUID to SendGoalRequest

Also make inner classes static.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add getStamp method to GoalResponseDefinition

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Partially revert "Add interfaces for action goal, result, and feedback"

Partially revert commit dd04614.

I don't think we need to aliases for the message types, but I'll add them back if they turn out to be useful.

* Parameterize goal request and response interfaces on action type

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Fix getGoalUuid implementation

It should return a List, since it is hashable.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* List<Byte> -> byte[]

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add ActionServer skeleton

* Add new action module with classes/interfaces related to the ActionServer
* Add methods for creating and removing ActionServers from a Node
* Implement dispose method for ActionServer.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add ActionServer creation logic and unit tests

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add action server logic for accepting and canceling goals

This changeset includes the following:
  * an implementation of the goal handle for action servers
  * action server integration with the base executor
  * action server integration with ROS nodes
  * unit tests for the action server, receiving goal and cancel requests

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* more cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor: move response enums into callback interfaces

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove TODO

The goal request already contains a getter for the goal ID.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Fix accident

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Alphabetize

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Refactor: replace 'getNumberOf*()' JNI functions with single 'getNumberOfEntites()' function

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Avoid string copy

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove extern C

Replace with 'namespace rcljava'

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove unnecessary synchronization

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Refactor access to goalCallback

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* typesafe request and response definitions

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Be more specific about template type for GoalCallback

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Make GoalHandleImpl inner class instead of static

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove synchronized from getHandle

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add TODO for Waitable interface

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Fix style

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add docs for createActionServer

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Switch from List to array

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Add cmake function for bundling source files into a jar
* Create and install source jar for rcljava

Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
ivanpauno and others added 20 commits May 17, 2022 15:36
* Add ACCEPT_AND_EXECUTE/ACCEPT_AND_DEFER to the result of an action goal callback.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

* Automatically transition from accepted to executing if ACCEPT_AND_EXECUTE was returned.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

* Expose an "execute()" method in goal callback.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
… to the feedback topic (osrf#60)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Julius Gyorfi <julius.s.gyorfi@nasa.gov>
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
* GoalHandle transitions should be able to set a result.
* Refactor GoalStatus enumeration.
* Add interface for ResultRequestDefinition, ResultResponseDefinition, FeedbackDefinition, GoalDefinition, ResultDefinition.
* Modify rcljava rosidl generator to implement those interfaces.
* Handle result requests.
* Handle goal handle terminal transitions.
* Cleanup goal handle status transition bindings.
* Take advantage of nested class.
* Publish status after terminal state transition.
* Handle expire goals.
* Add support for publishing feedback.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
…srf#73)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Cleanup service/client creation code

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

* Add methods to create a parameter client in Node

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Add overloads for spinOnce methods to include a timeout argument and call this when spinning in the future.

This fixes a bug where the future.get(...) call blocks forever, e.g. when waiting on a service response and the service becomes unavailable.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@jacobperron jacobperron requested review from esteve and ivanpauno May 17, 2022 23:05
@jacobperron
Copy link
Contributor Author

There is an unexpected test failure on linux CI:

 5: 1) testAdd(org.ros2.rcljava.client.ClientTest)
  5: java.lang.AssertionError: expected:<1> but was:<7>
  5: 	at org.junit.Assert.fail(Assert.java:88)
  5: 	at org.junit.Assert.failNotEquals(Assert.java:834)
  5: 	at org.junit.Assert.assertEquals(Assert.java:645)
  5: 	at org.junit.Assert.assertEquals(Assert.java:631)
  5: 	at org.ros2.rcljava.client.ClientTest.testAdd(ClientTest.java:131)
  5: 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  5: 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
  5: 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  5: 	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
  5: 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
  5: 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
  5: 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
  5: 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
  5: 	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
  5: 	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
  5: 	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
  5: 	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
  5: 	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
  5: 	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
  5: 	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
  5: 	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
  5: 	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
  5: 	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
  5: 	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
  5: 	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
  5: 	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
  5: 	at org.junit.runners.Suite.runChild(Suite.java:128)
  5: 	at org.junit.runners.Suite.runChild(Suite.java:27)
  5: 	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
  5: 	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
  5: 	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
  5: 	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
  5: 	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
  5: 	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
  5: 	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
  5: 	at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
  5: 	at org.junit.runner.JUnitCore.runMain(JUnitCore.java:77)
  5: 	at org.junit.runner.JUnitCore.main(JUnitCore.java:36)

There are also, not-so-unexpected, build issues on android (since we're not running CI on our fork). I'll resolve these before merging any commits contained in this PR.

@esteve
Copy link
Member

esteve commented May 18, 2022

@jacobperron first of all, thanks for the PR. But in all honesty, reviewing a 8K+ line diff is a lot of work, is there any chance this could be broken into separate features? Given that you and @ivanpauno both have write access to this repo, could you in the future do the work directly here? We did a sync months ago, and I thought that from then on, all development would be done upstream.

@jacobperron
Copy link
Contributor Author

Technically, either Ivan or I have already reviewed the changes in this PR (though I understand if we want to break this into separate smaller PRs). We've done previous PRs (#199 and #206) to start porting changes over, but there are a number of outstanding changes to upstream. I collected all remaining changes from our fork that we tested with Galactic and put them in this PR. I am hoping after this, there will be one more PR that adds changes for Humble/Rolling and then we can archive our fork.

@jacobperron
Copy link
Contributor Author

Thanks for opening up smaller PRs, @ivanpauno. I'll close this one in favor of those.

@jacobperron jacobperron closed this Aug 9, 2022
@ivanpauno ivanpauno deleted the jacob/osrf_ports_2 branch August 10, 2022 15:51
@bluecamel
Copy link

I don't see that there was a separate PR to port the action server from the osrf fork. Is that still a WIP?

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.

8 participants