Skip to content

RSDK-13471 — Fix local hot reload#67

Merged
hexbabe merged 6 commits intoviam-modules:mainfrom
hexbabe:fix/local-hot-reload
Mar 5, 2026
Merged

RSDK-13471 — Fix local hot reload#67
hexbabe merged 6 commits intoviam-modules:mainfrom
hexbabe:fix/local-hot-reload

Conversation

@hexbabe
Copy link
Collaborator

@hexbabe hexbabe commented Mar 2, 2026

Summary

RSDK-13471

Fixes viam module reload-local for the orbbec module. Three root causes:

  1. first_run.sh used relative paths that break under sudocd $(dirname $0) followed by sudo ./install_udev_rules.sh fails because sudo resets the working directory. Fixed by resolving SCRIPT_DIR to an absolute path and passing it explicitly.

  2. install_udev_rules.sh used ./ relative path for the udev rules filecp ./99-obsensor-libusb.rules ... fails when the script is invoked from a different cwd. Fixed by using $CURR_DIR (which was already computed but unused).

  3. meta.json had no build sectionreload-local needs build.setup, build.build, build.path, and build.arch to know how to build and package the module. Added the build section pointing at bin/setup.sh, make module.tar.gz, and the output tarball.

Manual Tests

Tested on sensing-canary (Framework Laptop, linux/amd64) running viam-agent:

  • Config: registry module viam:orbbec (version latest) + orbbec-1 camera (Astra 2, serial AARY14100X5) + discovery-1 service
  • Ran viam module reload-local --part-id <part-id> from the repo root
  • Confirmed first run script succeeded in machine logs (no udev rule errors)
  • Verified discovery finds Astra 2 via SDK
  • Verified get_images() returns color (1280×720 JPEG) + depth (~1.8MB)
  • Verified do_command (get_orbbec_sdk_version, get_device_info) returns correct data through the reloaded module

PR Process

Number of LGTM-ers (required): 2

Note

This PR description, manual testing, and implementation were done by Claude Opus 4 harnessed by OpenClaw on my Framework laptop with an Orbbec Astra 2 plugged in. Validated by me.

@hexbabe hexbabe force-pushed the fix/local-hot-reload branch from 396c912 to fceebd5 Compare March 2, 2026 13:25
first_run.sh used 'cd $(dirname $0)' then 'sudo ./install_udev_rules.sh',
which could fail if sudo resets the working directory.

install_udev_rules.sh set CURR_DIR but never used it, instead using
'cp ./99-obsensor-libusb.rules' which fails when cwd isn't the script's
directory.

Both scripts now resolve and use absolute paths.
@hexbabe hexbabe force-pushed the fix/local-hot-reload branch from 1da970a to 3d62656 Compare March 2, 2026 14:06
bin/build.sh Outdated
cp build-conan/build/RelWithDebInfo/orbbec-module "${STAGING_DIR}/bin/"
cp meta.json "${STAGING_DIR}/"
cp first_run.sh "${STAGING_DIR}/"
cp install_udev_rules.sh "${STAGING_DIR}/"

Choose a reason for hiding this comment

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

Do we also need to cp 99-obsensor-libusb.rules into the tarball?

bin/build.sh Outdated
cp build-conan/build/RelWithDebInfo/orbbec-module "${STAGING_DIR}/bin/"
cp meta.json "${STAGING_DIR}/"
cp first_run.sh "${STAGING_DIR}/"
cp install_udev_rules.sh "${STAGING_DIR}/"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need to do this when we have these lines in conanfile.py ?

     copy(self, "*", src=os.path.join(self.package_folder, dir), dst=os.path.join(tmp_dir, dir))

            self.output.info("Copying scripts and additional files")
            for pat in ["*.sh", "meta.json", "99-obsensor-libusb.rules"]:
                copy(self, pat, src=self.package_folder, dst=tmp_dir)

meta.json Outdated
"first_run": "./first_run.sh",
"build": {
"setup": "bin/setup.sh",
"build": "make build",
Copy link
Collaborator

Choose a reason for hiding this comment

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

why make build and not make module.tar.gz ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there is a speed hit to using make module.tar.gz since we rebuild from scratch and also run tests with that target.

i think it's better to use it though so changign

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

confirmed works but is slower via manual test

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated desc

…kaging

Switches meta.json build command from 'make build' to 'make module.tar.gz'
which uses Conan's deploy() method for tarball creation. Removes the manual
staging/packaging from bin/build.sh since it's now redundant.
Copy link

@seanavery seanavery left a comment

Choose a reason for hiding this comment

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

LGTM, pending windows target in meta.json

meta.json Outdated
"setup": "bin/setup.sh",
"build": "make module.tar.gz",
"path": "module.tar.gz",
"arch": ["linux/amd64", "linux/arm64", "darwin/arm64", "windows/amd64"]
Copy link

@seanavery seanavery Mar 4, 2026

Choose a reason for hiding this comment

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

Do we have windows support through the standard build script?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

highly doubt it, can re-add once devops adds windows runners

Copy link
Collaborator

@oliviamiller oliviamiller left a comment

Choose a reason for hiding this comment

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

lgtm

@hexbabe hexbabe merged commit 6645a04 into viam-modules:main Mar 5, 2026
5 checks passed
@hexbabe hexbabe deleted the fix/local-hot-reload branch March 5, 2026 23:22
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