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

Add more tests #9

Merged
merged 25 commits into from
Aug 26, 2024
Merged

Add more tests #9

merged 25 commits into from
Aug 26, 2024

Conversation

stefanscherzinger
Copy link
Member

@stefanscherzinger stefanscherzinger commented Aug 5, 2024

Steps

  • Add tests for the advertised topics
  • Add tests for the services
    • acknowledge
    • brake_test
    • fast_stop
    • gripper_info
    • prepare_for_shutdown
    • release_for_manual_movement
    • softreset
    • stop
  • Add tests for the actions
    • grip
    • grip_with_position
    • gripper_control
    • move_to_absolute_position
    • move_to_relative_position
    • release_workpiece

Also use `MultiThreadedExecutor` for the subscribing nodes.
The normal `spin()` lead to `ValueError: generator already executing in
ROS2 framework` errors when having them in the same test case.
This should reduce boilerplate code when testing each service interface.
We now have a cleaner separation between topics, services, and actions.
Also use a single fixture `running_driver` for testing topics, services,
and actions. This avoids brittle sleeps and makes sure that everything
is up and running before starting the actual tests.

The `test_startup.py` became obsolete with this.
Also add a function to set control bits in the dummy.
This avoids a lot of boilerplate code in tests.
Reporting the `success` bool flag is like an in-official convention.
There's no need for repeating the service's name.
In ROS2, the gripper has a single IP address at startup.
This service seems irrelevant for these driver's use cases and adds complexity
that's difficult to test and maintain.

We get it back once we really need this.
The driver _does_ reconnect automatically when the connection drops, but
it does it without assuming another IP.
The terminal log output was not easily accessible from client code.
We now return the collected information in a result string.
Correct sections that mentioned the `reconnect` and the `gripper_info` services.
Also simplify further action tests with a client wrapper.
This is easier to implement and test with the dummy.
We now return the actual data, not the metadata.
Also, we now interpret target position and target speed in the plc
commands as unsigned integers (position in um, speed in um/s) as stated
in the gripper's TCP/IP manual. Only the data in `self.data` need to be stored
and returned as floats.

Check this manual for further details:
[1]: https://stb.cloud.schunk.com/media/IM0046706.PDF
Also:
- Fix the `MotionProfile`s finish condition.
- Make tests for absolute positioning more watertight. Running several
  moves in a row was crucial in spotting the erroneous finish
  condition.
- Remove specific timeouts in the ROS2 action tests. It's more robust
  this way. If they fail, the CI fails with a reason.
- Make sure that the driver resets relevant status bits from previous
  requests.
The _EGU_ and _EGK_ versions differ in the action goal type.
Let's see if we can mimic both at some point.
Also drop setting the failure-related status bits 11 and 16.
We'll implement them later in the dummy once we properly test them in
our `test_actions.py`.
@stefanscherzinger stefanscherzinger force-pushed the add-more-tests branch 4 times, most recently from ae11ee5 to 3cf9578 Compare August 26, 2024 08:06
Also upgrade the `actions/checkout` version.
This makes them more robust against these unnecessary CI failures:

`assert 76.9995346069336 == 77.0 ± 7.7e-05`
Also add meaningful values for `max_grip_force`, `min_grip_force`,
`max_vel`, and `min_vel` (EGK 40).
@stefanscherzinger stefanscherzinger merged commit 3e5ed39 into main Aug 26, 2024
7 of 9 checks passed
@stefanscherzinger stefanscherzinger deleted the add-more-tests branch August 26, 2024 15:25
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.

1 participant