Skip to content

Conversation

cheukt
Copy link
Member

@cheukt cheukt commented Jul 24, 2023

  • Made the current example the complex example, added a single file sensor example. Single file does not use an init.py.

  • The arm example in jupyter notebook is now single file and does not depend on a kinematics.json file.

  • Made the env consistent between examples and jupyter notebook (requirements.txt) and removes the use of poetry (extra dependency)

  • For the jupyter example, reworded a bunch of stuff but mostly to make it consistent with the simple example and also point to the complex example when necessary.

  • tested by downloading a fresh copy of the repo on a pi and just adding it to the config.

still a lot of improvements that can be made, but wanted to get something out for now
cc @Fahmina @michaellee1019 @dannenberg

@cheukt cheukt requested a review from a team as a code owner July 24, 2023 19:38
Copy link
Contributor

@dannenberg dannenberg left a comment

Choose a reason for hiding this comment

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

huge step up. thrilled to see this!

Modular resources allows you to define custom components and services, and add them to your robot. Viam ships with many component types, but you're not limited to only using those types -- you can create your own using modules.

For more information, see the [documentation](https://docs.viam.com/program/extend/modular-resources/).
For more information, see the [documentation](https://docs.viam.com/program/extend/modular-resources/). For a simpler example, take a look at the [simple module example](https://github.com/viamrobotics/viam-python-sdk/tree/main/examples/simple_module), which only contains one custom resource model in one file.
Copy link
Contributor

Choose a reason for hiding this comment

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

it may be good to start with simple and then offer the grander example with custom proto

Copy link
Member Author

Choose a reason for hiding this comment

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

the readme here is part of the complex_module example folder, so I feel like unless we want to restructure the examples even more, it's the user's choice as to which example they start with (the line here is just to remind them there's something simpler)

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh. sorry. i'd misunderstood. thanks

cd `dirname $0`

# Create a virtual environment to run our code
VENV="venv"
Copy link
Contributor

Choose a reason for hiding this comment

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

is this the name of the venv? if so can we make that more explicit so as to not confuse it with the command proper? ex, VENV_NAME

Copy link
Member Author

Choose a reason for hiding this comment

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

ya, updated

Copy link
Member

@purplenicole730 purplenicole730 left a comment

Choose a reason for hiding this comment

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

This is amazing! LGTM

cd `dirname $0`

# Create a virtual environment to run our code
VENV_NAME="venv"
Copy link
Member

Choose a reason for hiding this comment

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

Does this not assume you have venv installed? and a virtual env named venv?

Copy link
Member Author

Choose a reason for hiding this comment

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

venv is baked into python 3.4 and up, so should be good for the versions of python we support.
I've used venv for myself for the longest time haha, is there a different name you like?

Copy link
Member

Choose a reason for hiding this comment

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

AH sorry this is creating a virtual env. I guess then it assumes that there isn't already a virtual env named venv? What would happen if a user already have a venv?

Copy link
Member

Choose a reason for hiding this comment

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

NVM, it creates it in this directory, so there should be no collisions.

cd `dirname $0`

# Create a virtual environment to run our code
VENV_NAME="venv"
Copy link
Member

Choose a reason for hiding this comment

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

AH sorry this is creating a virtual env. I guess then it assumes that there isn't already a virtual env named venv? What would happen if a user already have a venv?

cd `dirname $0`

# Create a virtual environment to run our code
VENV_NAME="venv"
Copy link
Member

Choose a reason for hiding this comment

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

NVM, it creates it in this directory, so there should be no collisions.

@michaellee1019
Copy link
Member

LGTM! This simple example will help a lot. Thanks for reorganizing it.

Copy link
Contributor

@npentrel npentrel left a comment

Choose a reason for hiding this comment

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

Some small suggestions to improve wording but looks good! Sorry for the delay!

cheukt and others added 8 commits July 26, 2023 12:29
Co-authored-by: Naomi Pentrel <5212232+npentrel@users.noreply.github.com>
Co-authored-by: Naomi Pentrel <5212232+npentrel@users.noreply.github.com>
Co-authored-by: Naomi Pentrel <5212232+npentrel@users.noreply.github.com>
Co-authored-by: Naomi Pentrel <5212232+npentrel@users.noreply.github.com>
Co-authored-by: Naomi Pentrel <5212232+npentrel@users.noreply.github.com>
Co-authored-by: Naomi Pentrel <5212232+npentrel@users.noreply.github.com>
Co-authored-by: Naomi Pentrel <5212232+npentrel@users.noreply.github.com>
@cheukt cheukt merged commit 5df6116 into viamrobotics:main Jul 26, 2023
@cheukt cheukt deleted the arm-stuff branch July 26, 2023 16:40
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.

6 participants