Skip to content

Create Dummy ROS2 Nodes for MoveAbovePlate, Integrates into App #34

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

Merged
merged 1 commit into from
May 12, 2023

Conversation

amalnanavati
Copy link
Contributor

@amalnanavati amalnanavati commented May 5, 2023

[Describe this pull request. Link to relevant GitHub issues, if any.]

In service of #27 .

This PR does the following:

  • Creates ROS2 messages for the actions and services that the web app will use. These changes are in affiliated PR ada_feeding#19.
  • Implements a dummy ROS actions for MoveAbovePlate. This dummy action sleeps for 2.5 secs during planning, and 10 secs during motion.
  • Has the web app call this action where appropriate, and render its feedback/results.
  • In order to do the above, this PR also implements the generalizes the Footer to take in a pause state and pause/resume callbacks from the main app page, as opposed to creating it locally in that component.

[Explain how this pull request was tested, including but not limited to the below checkmarks.]

I first tested the ROS Action in isolation, by ensuring that it behaved as expected when:

  • It is called and is allowed to finish uninterrupted. (Should return periodic feedback and then success status).
  • It is called and then canceled during planning. (Should return periodic feedback and then cancel status).
  • It is called and then canceled during motion. (Should return periodic feedback and then cancel status).
  • A second goal is sent while first goal is executing. (Should reject the second goal.)
  • A second goal is sent after canceling the first goal during planning. (Should accept the second goal.)
  • A second goal is sent after canceling the first goal during motion. (Should accept the second goal.)
  • A second goal is sent after the first goal succeeds. (Should accept the second goal.)
  • Planning fails (returns success==false). (Should return planning failure.)
  • Motion fails (returns success==false). (Should return motion failure.)

I then tested the web app's usage of the ROS action by, for the MoveAbovePlate page:

  • Checking that, in normal usage, it calls the ROS action exactly once upon page loading, properly renders the feedback, and moves onto the next state.
  • Checking that "pause" cancels the ROS action, and "resume" re-calls it. Verifying that all these call are made exactly once.
  • Verifying that if the page is refreshed it cancels the ROS action.

Before creating a pull request

  • Format code with npm run format
  • Thoroughly test your code's functionality, including unintended uses.
  • [N/A, no visual changes that are intended to be in the final app] Thoroughly test your code's responsiveness by rendering it on different devices, browsers, etc.
  • [N/A, doesn't change the user flow] Consider the user flow between states that this feature introduces, consider different situations that might occur for the user, and ensure that there is no way for the user to get stuck in a loop.

Before merging a pull request

  • Squash all your commits into one (or Squash and Merge)

@amalnanavati amalnanavati force-pushed the amaln/ros2_dummy_nodes branch from bffa484 to 564644c Compare May 5, 2023 20:14
@amalnanavati amalnanavati changed the title Create Dummy ROS2 Nodes for Robot Motion, Integrate into App Create Dummy ROS2 Nodes for MoveAbovePlate, Integrates into App May 5, 2023
@amalnanavati amalnanavati requested a review from taylorkf May 5, 2023 20:19
Copy link
Contributor

@taylorkf taylorkf left a comment

Choose a reason for hiding this comment

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

Everything ran great the first time, letting MoveAbovePlate run unpaused to the end. I ran into this error after both pausing and refreshing the page before MoveAbovePlate was completed. Both of these caused MoveAbovePlate to cancel and restart, but when the action was completed it moved to a blank screen. Now, when I restart the app using npm start, it begins MoveAbovePlate and then proceeds to the blank screen.
terminal-errors
Desktop-screenshot

@amalnanavati
Copy link
Contributor Author

Hmm, I am not sure what caused that error. I just tried: (1) running it normally; (2) pausing and resuming; (3) pausing then refreshing; (4) refreshing then pausing; (5) pausing then resuming then refreshing. It all moved to the next state as expected.

@taylorkf can you try the following:

  1. Clear your cookies to let the app re-start from normal.
  2. Re-run the app, see if you can re-create the error, and document the steps required to re-create it.
  3. If you get the error again, click on some of the links in the console to isolate where in the code the error is coming from, and send that to me?

(The errors in rclpy are errors I've seen before, I think it is due to the fact that rosbridge_websocket doesn't officially support ROS actions. I've never had that error affect functionality though.)

@taylorkf
Copy link
Contributor

taylorkf commented May 9, 2023

Ok, after testing multiple times, it appears that on my computer this breaks with a refresh at any point in time. The links trace the error to the shown line of code. From a brief google, it seems that this may be due to an unused useState? (https://stackoverflow.com/questions/53472795/uncaught-error-rendered-fewer-hooks-than-expected-this-may-be-caused-by-an-acc)
Desktop-screenshot (1)

@amalnanavati
Copy link
Contributor Author

It looks like you are using old code -- see MovingAbovePlate on the branch associated with this PR. Your screenshot shows an extra line that gets mealState and an extra if statement (line 2427 in your screenshot) that no longer exists in the code. That if statement is what is causing this issue.

Can you pull again and re-build?

@amalnanavati amalnanavati requested a review from taylorkf May 9, 2023 17:55
Copy link
Contributor

@taylorkf taylorkf left a comment

Choose a reason for hiding this comment

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

This looks great. I added a few comments that don't need to be addressed in this PR, just general info.

- Implemented and tested the MoveAbovePlate dummy ROS action
- Integrated it with the web app and thoroughly tested it
- Made the Footer take in paused/setPaused and callback functions from the page it is on
- Updated README
@amalnanavati amalnanavati force-pushed the amaln/ros2_dummy_nodes branch from 92427d8 to c74490c Compare May 12, 2023 02:02
@amalnanavati amalnanavati merged commit e43c464 into main May 12, 2023
@amalnanavati amalnanavati deleted the amaln/ros2_dummy_nodes branch May 12, 2023 02:05
@amalnanavati amalnanavati mentioned this pull request May 20, 2023
6 tasks
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