Skip to content

[RSDK-8045] connect to machine from viam client - #685

Merged
Naveed Jooma (njooma) merged 5 commits into
viamrobotics:mainfrom
njooma:RSDK-8045/simul-auth
Jul 17, 2024
Merged

[RSDK-8045] connect to machine from viam client#685
Naveed Jooma (njooma) merged 5 commits into
viamrobotics:mainfrom
njooma:RSDK-8045/simul-auth

Conversation

@njooma

Copy link
Copy Markdown
Member

Adds the ability to connect to a machine with an already-connected Viam client

@purplenicole730 Nicole Jung (purplenicole730) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One question about main parts, and a quick type fix. Otherwise, looks good to me! Thank you for adding a viam_client test!

Comment thread src/viam/services/service_base.py
Comment thread src/viam/app/viam_client.py Outdated

if id is not None and address is None:
parts = await self.app_client.get_robot_parts(id)
main_part = [p for p in parts if p.main_part][0]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know how main parts work; are there multiple main parts with the same address?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There should be one main part per machine, and this will find it.

Comment thread src/viam/app/viam_client.py Outdated

if id is not None and address is None:
parts = await self.app_client.get_robot_parts(id)
main_part = [p for p in parts if p.main_part][0]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(minor) not sure if we expect to have enough parts for this to matter, but I think this could be slightly cleaner/more efficient as main_part = next(p for p in parts if p.main_part).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Tested it, and it's about 25% faster to do the iterator method so I updated

@njooma
Naveed Jooma (njooma) merged commit be98e2d into viamrobotics:main Jul 17, 2024
@njooma
Naveed Jooma (njooma) deleted the RSDK-8045/simul-auth branch July 17, 2024 18:05
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.

3 participants