Module for graphnav by heuristicus · Pull Request #44 · rai-opensource/spot_wrapper

heuristicus · 2023-06-29T15:36:22Z

Potentially needs to integrate Fast graph uploading #28 Add graph download/upload/clear methods #30 Fix logging output of graph_nav_util.py #31 Add localization interface and cancelable navigation methods #40, but hopefully hold those until this has been merged?
Partial regression because some of the functions used try_claim: navigate_to, _navigate_to, and navigate_route

* use pytest instead * add pytest dependency * fail test on purpose * remove failing test

…o main

…pot_ros2 (rai-opensource#7) Co-authored-by: Andrew Messing <129519955+amessing-bdai@users.noreply.github.com>

* simple led brightness control, only able to set all leds to same value * Add power control, but unclear if it is actually possible to set power for aux and external mic * most basic functional image stream publisher with webrtc * add compositor to handle IR and webrtc stream selection with services Add timestamp for the webrtc images Add compressed version of the webrtc image stream * Add health wrapper, move body of robotToLocalTime out of spot wrapper object robotToLocalTime now takes the timestamp and a robot object, which allows it to be used by the spot cam wrapper as well. * add handler and wrapper for audio commands * update webrtc_client to 3.2 version * add stream quality wrapper and ros handler * initial implementation of ptz wrapper and handler, can list ptzs * ptz handler publishes position and velocity of ptzs, can set position and velocity * add egg info to gitignore

jbarry-bdai · 2023-08-01T16:31:44Z

@kaiyu-zheng can you either test this or work with @smayorga-bdai to test this? we're starting to get blocked on this so let's try and get it in.

smayorga-bdai · 2023-08-02T17:04:40Z

I think theres some regressions. I was able to run our test_graphnav_services script on our internal repo "main" but after checking out heuristicus/modular-graphnav I got the following errors when uploading the graph.

[spot_ros2-1] [INFO] [1690995381.627716334] [Chara.spot_wrapper]: Loaded graph has 111 waypoints and 118 edges
[spot_ros2-1] [ERROR] [1690995381.628465313] [Chara.spot_ros2]: Exception Error:'SpotWrapper' object has no attribute '_current_waypoint_snapshots'; 
[spot_ros2-1]  Traceback (most recent call last):
[spot_ros2-1]   File ".../spot_driver/spot_ros2.py", line 1959, in handle_graph_nav_upload_graph
[spot_ros2-1]     self.spot_wrapper._upload_graph_and_snapshots(request.upload_filepath)
[spot_ros2-1]   File ".../spot_wrapper/wrapper.py", line 1704, in _upload_graph_and_snapshots
[spot_ros2-1]     self._current_waypoint_snapshots[
[spot_ros2-1] AttributeError: 'SpotWrapper' object has no attribute '_current_waypoint_snapshots'
[spot_ros2-1]

jbarry-bdai · 2023-08-02T17:38:54Z

I think theres some regressions. I was able to run our test_graphnav_services script on our internal repo "main" but after checking out heuristicus/modular-graphnav I got the following errors when uploading the graph.

[spot_ros2-1] [INFO] [1690995381.627716334] [Chara.spot_wrapper]: Loaded graph has 111 waypoints and 118 edges
[spot_ros2-1] [ERROR] [1690995381.628465313] [Chara.spot_ros2]: Exception Error:'SpotWrapper' object has no attribute '_current_waypoint_snapshots'; 
[spot_ros2-1]  Traceback (most recent call last):
[spot_ros2-1]   File ".../spot_driver/spot_ros2.py", line 1959, in handle_graph_nav_upload_graph
[spot_ros2-1]     self.spot_wrapper._upload_graph_and_snapshots(request.upload_filepath)
[spot_ros2-1]   File ".../spot_wrapper/wrapper.py", line 1704, in _upload_graph_and_snapshots
[spot_ros2-1]     self._current_waypoint_snapshots[
[spot_ros2-1] AttributeError: 'SpotWrapper' object has no attribute '_current_waypoint_snapshots'
[spot_ros2-1]

Oh yeah we need to update spot_ros2 to work with this PR (@bhung-bdai )... Can we not test it until that is done @kaiyu-zheng ?

jbarry-bdai · 2023-08-02T17:39:28Z

@baxelrod-bdai fyi

heuristicus · 2023-08-02T18:03:32Z

The main change needed should be to call functions using graphnav with spot_wrapper.spot_graph_nav rather than just the wrapper, and there may be functions whose name changed, but I don't think so.

bhung-bdai · 2023-08-02T18:14:21Z

I think theres some regressions. I was able to run our test_graphnav_services script on our internal repo "main" but after checking out heuristicus/modular-graphnav I got the following errors when uploading the graph.
[spot_ros2-1] [INFO] [1690995381.627716334] [Chara.spot_wrapper]: Loaded graph has 111 waypoints and 118 edges
[spot_ros2-1] [ERROR] [1690995381.628465313] [Chara.spot_ros2]: Exception Error:'SpotWrapper' object has no attribute '_current_waypoint_snapshots'; 
[spot_ros2-1]  Traceback (most recent call last):
[spot_ros2-1]   File ".../spot_driver/spot_ros2.py", line 1959, in handle_graph_nav_upload_graph
[spot_ros2-1]     self.spot_wrapper._upload_graph_and_snapshots(request.upload_filepath)
[spot_ros2-1]   File ".../spot_wrapper/wrapper.py", line 1704, in _upload_graph_and_snapshots
[spot_ros2-1]     self._current_waypoint_snapshots[
[spot_ros2-1] AttributeError: 'SpotWrapper' object has no attribute '_current_waypoint_snapshots'
[spot_ros2-1] 
Oh yeah we need to update spot_ros2 to work with this PR (@bhung-bdai )... Can we not test it until that is done @kaiyu-zheng ?

I'm admittedly confused to the status of this. When I talked to @amessing-bdai, he mentioned there were multiple PRs blocking other things throughout this whole process. That's mainly why I've been hestitant to test it; I just don't know exactly what is dependent on what, so I don't know what order I'm supposed to be evaluating changes.

jbarry-bdai · 2023-08-02T19:11:48Z

@bhung-bdai My understanding of what's going on (from @amessing-bdai who is on vacation) is that:

We're not accepting any new PRs involving graph_nav into spot wrapper or spot ros2 until this PR goes in. That is blocking several PRs
This PR can't go in until we test it and update spot ros2 to call it correctly. I originally thought we could test it without updating spot ros2 but actually thinking about it more I don't see how.

So I think we need to:

Update spot ros2 to be compatible with this PR
Test that PR and this PR together
Commit them at essentially the same time

I'm hoping the spot ros2 change is somewhat trivial but I haven't actually looked at it...

bhung-bdai · 2023-08-02T19:14:51Z

I'll talk to Kaiyu about it. Andrew mentioned that he had a HIL test for graphnav. Brandon

…

On Wed, Aug 2, 2023, 3:11 PM jbarry-bdai ***@***.***> wrote: @bhung-bdai <https://github.com/bhung-bdai> My understanding of what's going on (from @amessing-bdai <https://github.com/amessing-bdai> who is on vacation) is that: - We're not accepting any new PRs involving graph_nav into spot wrapper or spot ros2 until this PR goes in. That is blocking several PRs - This PR can't go in until we test it and update spot ros2 to call it correctly. I originally thought we could test it without updating spot ros2 but actually thinking about it more I don't see how. So I think we need to: 1. Update spot ros2 to be compatible with this PR 2. Test that PR and this PR together 3. Commit them at essentially the same time I'm hoping the spot ros2 change is somewhat trivial but I haven't actually looked at it... — Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BAW5F4P4GP4RZBI2GT3PU7DXTKQ77ANCNFSM6AAAAAAZYWB6UE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

jbarry-bdai · 2023-08-02T19:24:44Z

@bhung-bdai he does and the above is the output I believe from running that test (which @smayorga-bdai did with @kaiyu-zheng 's guidance so he can help test this out now!). The problem is that it relies on spot ros2, which isn't compatible with this PR :(

bhung-bdai · 2023-08-02T19:28:46Z

I can get on this. If it was like the spot_images modular change, then it should be fairly simple to change spot_ros2 to work with it. Brandon

…

On Wed, Aug 2, 2023, 3:24 PM jbarry-bdai ***@***.***> wrote: @bhung-bdai <https://github.com/bhung-bdai> he does and the above is the output I believe from running that test (which @smayorga-bdai <https://github.com/smayorga-bdai> did with @kaiyu-zheng <https://github.com/kaiyu-zheng> 's guidance so he can help test this out now!). The problem is that it relies on spot ros2, which isn't compatible with this PR :( — Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BAW5F4MAMIGEFKZNQMAYTWLXTKSQRANCNFSM6AAAAAAZYWB6UE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

heuristicus · 2023-08-03T14:58:41Z

@bhung-bdai it looks like the last merge of the main branch into this branch unintentionally reintroduced some of the graphnav functions to the wrapper. I will revert and merge correctly.

bhung-bdai · 2023-08-03T15:01:40Z

Ah, that explains a lot. Thanks! I apologize for the spam. Brandon

…

On Thu, Aug 3, 2023, 10:58 AM Michal Staniaszek ***@***.***> wrote: @bhung-bdai <https://github.com/bhung-bdai> it looks like the last merge of the main branch into this branch unintentionally reintroduced some of the graphnav functions to the wrapper. I will revert and merge correctly. — Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BAW5F4J6333O6GAOKLMLABDXTO4CZANCNFSM6AAAAAAZYWB6UE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

heuristicus · 2023-08-03T15:05:52Z

Should be corrected now.

…pper.py. Will get rid of these comments upon final revision

bhung-bdai · 2023-08-03T21:04:11Z

Localization HIL test works with the update spot_ros2. I would like to test navigation functions before I approve it though.

…ware

bhung-bdai · 2023-08-03T23:10:27Z

Tested our HIL test with spot_ros2 and spot_wrapper, also tested to make sure the navigation and graph list + upload functions still work. Added a minor bug fix.

bhung-bdai

LGTM

mpickett-bdai · 2023-08-04T14:12:26Z

+        assert self.name_to_id["Node1"] == "ABCDE"
+        assert self.name_to_id["Node2"] == "DE"
+
+    def test_two_waypoints_with_edge_and_localization(self):


Is there a difference between this test and the test above? I'm not as familiar with this code so maybe I'm missing something.

I see it now with the self.localization_id set differently.

mpickett-bdai

LGTM s

jeremysee2 and others added 30 commits April 18, 2023 18:48

refactored code into modules

adfb0b1

refactored image service

76f4899

cleanup imports

f026a3e

add unit tests for some graph_nav_util functions

88ae203

pip install requirements

be35078

typing changes

86a604f

absolute paths

f4e9e43

clone to specific directory

03c5997

check file structure for CI

89213d8

check files

1ffab67

check files

860c796

add test script

0097875

install spot wrapper

5f04353

install script

7a9811b

remove asyncimageservice

15addcd

use robot_params to share state variables

3fa6430

Pytest replacement (#1)

55c68a1

* use pytest instead * add pytest dependency * fail test on purpose * remove failing test

replace print() with self._logger.error()

cf262e4

image publishing works well

4928997

moved SPOT_CLIENT_NAME

0c2bb45

static typing for claim()

6779aa5

Merge branch 'main' of https://github.com/jeremysee2/spot_wrapper int…

53fd6bf

…o main

black formatting

4881d38

comments and passing error feedback

392da89

remove spot_config

22d1084

use fstring for short code, move wrench from msg function to class body

73d88be

Add changes from [SW-62] Elements for publishing the hand camera in s…

c473eef

…pot_ros2 (rai-opensource#7) Co-authored-by: Andrew Messing <129519955+amessing-bdai@users.noreply.github.com>

fix bad indent after cherry-pick

9b20fe1

fix formatting

2ea754a

heuristicus added 2 commits July 22, 2023 12:27

Merge branch 'main' into modular-graphnav

59ad2d2

Merge branch 'main' into modular-graphnav

9b0eedf

heuristicus mentioned this pull request Aug 3, 2023

Modular graphnav heuristicus/spot_wrapper#6

Closed

Merge branch 'main' into modular-graphnav

ae55ef4

heuristicus force-pushed the modular-graphnav branch from b16ddc3 to ae55ef4 Compare August 3, 2023 15:04

Commented out some decorator functions which are no longer in the wra…

6f88ad1

…pper.py. Will get rid of these comments upon final revision

Updated some minor bug fixes with modular-graphnav and tested on hard…

1471332

…ware

bhung-bdai approved these changes Aug 3, 2023

View reviewed changes

bhung-bdai mentioned this pull request Aug 3, 2023

Edited spot_ros2 to work with new graphnav rai-opensource/spot_ros2#120

Merged

mpickett-bdai reviewed Aug 4, 2023

View reviewed changes

mpickett-bdai approved these changes Aug 4, 2023

View reviewed changes

formatting

54d6363

baxelrod-bdai approved these changes Aug 4, 2023

View reviewed changes

bhung-bdai merged commit d96bdbf into rai-opensource:main Aug 4, 2023

mhidalgo-rai mentioned this pull request Sep 23, 2024

GraphNav fixes rai-opensource/spot_ros2#486

Closed

Conversation

heuristicus commented Jun 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbarry-bdai commented Aug 1, 2023

Uh oh!

smayorga-bdai commented Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbarry-bdai commented Aug 2, 2023

Uh oh!

jbarry-bdai commented Aug 2, 2023

Uh oh!

heuristicus commented Aug 2, 2023

Uh oh!

bhung-bdai commented Aug 2, 2023

Uh oh!

jbarry-bdai commented Aug 2, 2023

Uh oh!

bhung-bdai commented Aug 2, 2023 via email

Uh oh!

jbarry-bdai commented Aug 2, 2023

Uh oh!

bhung-bdai commented Aug 2, 2023 via email

Uh oh!

heuristicus commented Aug 3, 2023

Uh oh!

bhung-bdai commented Aug 3, 2023 via email

Uh oh!

heuristicus commented Aug 3, 2023

Uh oh!

bhung-bdai commented Aug 3, 2023

Uh oh!

bhung-bdai commented Aug 3, 2023

Uh oh!

bhung-bdai left a comment

Choose a reason for hiding this comment

Uh oh!

mpickett-bdai Aug 4, 2023

Choose a reason for hiding this comment

Uh oh!

mpickett-bdai Aug 4, 2023

Choose a reason for hiding this comment

Uh oh!

mpickett-bdai left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

heuristicus commented Jun 29, 2023 •

edited

Loading

smayorga-bdai commented Aug 2, 2023 •

edited

Loading