Skip to content

Conversation

@fdurchdewald
Copy link
Contributor

This pull request includes the 'robot_state_helper' node, which has been ported from the ROS1 to the ROS2 version of the driver.

@fmauch fmauch self-requested a review February 26, 2024 10:44
@urfeex
Copy link
Member

urfeex commented Feb 6, 2025

@fdurchdewald I hope you don't mind me coming back to this. As far as I remember you told me that there were still some unresolved problems with this, but I just rebased and tested this and it seems to to exactly what it is supposed to do.

@urfeex urfeex added this to the ROS1 feature parity milestone Feb 6, 2025
@urfeex urfeex requested review from urfeex and removed request for fmauch February 6, 2025 09:29
@codecov
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 3.74%. Comparing base (1b121b7) to head (def8650).
Report is 400 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##            main    #933      +/-   ##
========================================
+ Coverage   3.59%   3.74%   +0.15%     
========================================
  Files         13     397     +384     
  Lines        947   43840   +42893     
  Branches     152    6440    +6288     
========================================
+ Hits          34    1640    +1606     
- Misses       843   42057   +41214     
- Partials      70     143      +73     
Flag Coverage Δ
unittests 3.74% <ø> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@URJala
Copy link
Collaborator

URJala commented Feb 12, 2025

Tested on a real robot, and switching between Running and Idle seems to work well, but there are couple of things:

  1. Switching to Power Off from any other state causes the helper node to abort the goal with the message:
    Robot reached higher mode than requested during recovery. This either means that something went wrong or that a higher mode was requested from somewhere else (e.g. the teach pendant.)
    and then crash with the message:
    goal_handle attempted invalid transition from state ABORTED with event SUCCEED
    requiring a restart of the driver to use it again.

  2. If you try to use it while not in remote control mode, it will print an explanation that you should switch to remote control and reconnect, but it does not abort the action that requested the state change, so it just hangs forever.

  3. When changing from running to idle, it seems to completely restart the robot, going through many state changes. But that might just be the way to put the robot in idle, I am not sure.

Otherwise it seems to work well with resuming and stopping programs.
A couple of other notes, that might be more review-related:
It might be nice to define the available modes as constants in the action definition, and I am not quite sure I understand the need for both a stop_program and a play_program option, when a program cannot be played in anything other than running anyway, and therefore neither option has any effect in other modes than running (as far as i can tell).

@urfeex
Copy link
Member

urfeex commented Feb 12, 2025

@URJala Thanks for testing this. I've refactored the code to address some of your findings.

  1. Switching to Power Off from any other state causes the helper node to abort the goal with the message

That should be gone now.

  1. If you try to use it while not in remote control mode, it will print an explanation that you should switch to remote control and reconnect, but it does not abort the action that requested the state change, so it just hangs forever.

I assume this is gone, as well, but I cannot test it currently.

  1. When changing from running to idle, it seems to completely restart the robot, going through many state changes. But that might just be the way to put the robot in idle, I am not sure.

To my knowledge, the dashboard server doesn't contain a "engage brakes" call. So, we have to go through power_off for that transition.

Regarding stop and play: Did you read what I wrote into the documentation?

@URJala
Copy link
Collaborator

URJala commented Feb 13, 2025

Tested it again, and it seems to work perfectly now, couldnt find any of the problems from yesterday.

@urfeex urfeex merged commit d4947aa into UniversalRobots:main Mar 4, 2025
12 of 14 checks passed
mergify bot pushed a commit that referenced this pull request Mar 4, 2025
---------

Co-authored-by: Felix Durchdewald <[email protected]>
Co-authored-by: Felix Exner <[email protected]>
(cherry picked from commit d4947aa)

# Conflicts:
#	ur_robot_driver/CMakeLists.txt
#	ur_robot_driver/doc/index.rst
#	ur_robot_driver/doc/usage/startup.rst
urfeex added a commit that referenced this pull request Mar 4, 2025
---------

Co-authored-by: Felix Durchdewald <[email protected]>
Co-authored-by: Felix Exner <[email protected]>
@urfeex urfeex mentioned this pull request Mar 5, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants