Skip to content

Commit

Permalink
[RSDK-9936] Get the CI system to build the module again (#30)
Browse files Browse the repository at this point in the history
Tried on my Orin Nano: seems to work!

Changes include:
- Github has upgraded its CI runners to Python 3.13. This PR upgrades `pillow` and `numpy` to py3.13-compatible versions.
- Print more diagnostic info in the github action that installs dependencies. If the dependencies install correctly, no one will notice it, and if they don't install correctly, this will be super useful.
- The py3.13 linter has a new check for too many positional arguments, which the Viam API fails. We need to faithfully implement the API, so I disabled that check on the line that violates it.
- Half the tests were getting skipped without installing `pytest-asyncio`, so I added that in.
- Most of requirements.txt was not pinned: you and I might be running very different versions of each dependency, because we get whatever the latest one was when we installed. This can be dangerous if one of those packages makes a breaking change. So, I pinned everything to what I installed. I'm open to other versions instead, but think it's important to pin them all.
- I added a .gitignore so that running `git status` shows the repo as clean even after running the tests. Why didn't other people have this issue in the past? I wonder if I was supposed to set things up differently...
- and I added more to .gitignore so that the repo is clean even after running `make setup`. I could be wrong about this: maybe you want the presence of those build artifacts front-and-center.
- When running as a local module, viam-server expects meta.json to be in the same directory as your .tar.gz file. So, I symlinked it into dist/ in build.sh. I also forced the symlink to be created, so that it doesn't fail if you run `make setup` a second time. That has a bit of a smell, but I couldn't think of a better approach. Any ideas?
- The Python SDK renamed `SUBTYPE` to `API` recently, so I renamed to match.

There's still something slightly off: `make dist/archive.tar.gz` failed with errors about a nonexistent file, until I ran `make setup`. Since then, `make dist/archive.tar.gz` doesn't run because everything is up-to-date even though I edited the code. but perhaps that's a problem for another day.

(I also had spurious errors about being unable to open /tmp/something/libpython3.10.so, but then they spontaneously disappeared 🤷 )


* upgrade packages for Python 3.13 support. Print more diagnostics when installing requirements in github actions

* make the linter happy

* get the tests to all run

* add a .gitignore

* pin versions of everything, to prevent versioning problems on new installs

* Running make setup shouldn't make the repo dirty

* symlink meta.json into dist/ for ease of running as a local module

* rename SUBTYPE -> API, per changes in the SDK

* force the symlink even if it already exists

* add .venv to the .gitignore, too
  • Loading branch information
penguinland authored Feb 10, 2025
1 parent 42a710e commit 6601b24
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 10 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ jobs:
- uses: actions/checkout@v3

- name: Install dependencies
run: pip install -r requirements.txt
run: |
python --version
pip install --upgrade pip
pip --version
pip install -r requirements.txt
- name: Run lint
run: make lint
Expand Down
7 changes: 7 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
__pycache__
.venv

# Artifacts created from running `make setup`
build/
dist/
main.spec
2 changes: 2 additions & 0 deletions build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,7 @@ echo installing dependencies from requirements.txt
$VIRTUAL_ENV/bin/pip install -r requirements.txt -U
source $VIRTUAL_ENV/bin/activate
$PYTHON -m PyInstaller --onefile --hidden-import="googleapiclient" --add-data="./src:src" src/main.py
# When running as a local module, we need meta.json to be in the same directory as the module.
ln -sf ../meta.json dist
tar -czvf dist/archive.tar.gz dist/main

13 changes: 7 additions & 6 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
pillow == 10.0.1
numpy == 1.25.0
viam-sdk
pyinstaller
pylint
pytest
pillow == 11.0.0
numpy == 1.26.4
viam-sdk==0.39.0
pyinstaller==6.11.1
pylint==3.3.4
pytest==8.3.4
pytest-asyncio==0.25.3
opencv-python == 4.9.0.80
4 changes: 2 additions & 2 deletions src/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ async def main():
registry before the module adds the resource model.
"""
Registry.register_resource_creator(
Vision.SUBTYPE,
Vision.API,
MotionDetector.MODEL,
ResourceCreatorRegistration(
MotionDetector.new_service, MotionDetector.validate_config
),
)
module = Module.from_args()

module.add_model_from_registry(Vision.SUBTYPE, MotionDetector.MODEL)
module.add_model_from_registry(Vision.API, MotionDetector.MODEL)
await module.start()


Expand Down
3 changes: 2 additions & 1 deletion src/motion_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ async def get_properties(
object_point_clouds_supported=False,
)

async def capture_all_from_camera(
# The linter doesn't like the vision service API, which we can't change.
async def capture_all_from_camera( # pylint: disable=too-many-positional-arguments
self,
camera_name: str,
return_image: bool = False,
Expand Down

0 comments on commit 6601b24

Please sign in to comment.