From 6a9069fa0fbc111fc93d393d3ee1b6c3da6faeca Mon Sep 17 00:00:00 2001 From: sean yu Date: Sun, 1 Mar 2026 21:43:05 -0500 Subject: [PATCH 1/9] =?UTF-8?q?RSDK-13471=20=20=20=20=E2=80=94=20=20=20=20?= =?UTF-8?q?Fix=20local=20hot=20reload?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- meta.json | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/meta.json b/meta.json index e91fae27..f5098bc5 100644 --- a/meta.json +++ b/meta.json @@ -19,5 +19,11 @@ } ], "entrypoint": "bin/orbbec-module", - "first_run": "./first_run.sh" + "first_run": "./first_run.sh", + "build": { + "setup": "bin/setup.sh", + "build": "make build", + "path": "module.tar.gz", + "arch": ["linux/amd64"] + } } From 58c4bd1155bc4b147e2635a8bb677b5969476863 Mon Sep 17 00:00:00 2001 From: sean yu Date: Sun, 1 Mar 2026 21:58:19 -0500 Subject: [PATCH 2/9] chore: add all supported architectures to build section --- meta.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meta.json b/meta.json index f5098bc5..93a25f38 100644 --- a/meta.json +++ b/meta.json @@ -24,6 +24,6 @@ "setup": "bin/setup.sh", "build": "make build", "path": "module.tar.gz", - "arch": ["linux/amd64"] + "arch": ["linux/amd64", "linux/arm64", "darwin/arm64", "windows/amd64"] } } From fceebd5ccb8778294b2de7ac806c5e456cfabef3 Mon Sep 17 00:00:00 2001 From: sean yu Date: Mon, 2 Mar 2026 08:24:52 -0500 Subject: [PATCH 3/9] fix: package binary and udev rules into module tarball in build.sh --- bin/build.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/bin/build.sh b/bin/build.sh index 2ba82977..96219dd7 100755 --- a/bin/build.sh +++ b/bin/build.sh @@ -38,3 +38,14 @@ conan build . \ -s:a build_type=Release \ -s:a "&:build_type=RelWithDebInfo" \ -s:a compiler.cppstd=17 + +# Package the module binary, meta.json, and first_run.sh into module.tar.gz +# so that `viam module reload-local` picks it up correctly (see meta.json build.path). +STAGING_DIR="$(mktemp -d)" +mkdir -p "${STAGING_DIR}/bin" +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}/" +tar czf module.tar.gz -C "${STAGING_DIR}" . +rm -rf "${STAGING_DIR}" From 3d62656886a788f64db4a563093d599b892fd89f Mon Sep 17 00:00:00 2001 From: sean yu Date: Mon, 2 Mar 2026 08:53:01 -0500 Subject: [PATCH 4/9] fix: use absolute paths in first_run.sh and install_udev_rules.sh 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. --- bin/build.sh | 2 -- first_run.sh | 4 ++-- install_udev_rules.sh | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/bin/build.sh b/bin/build.sh index 96219dd7..b2bcd62e 100755 --- a/bin/build.sh +++ b/bin/build.sh @@ -39,8 +39,6 @@ conan build . \ -s:a "&:build_type=RelWithDebInfo" \ -s:a compiler.cppstd=17 -# Package the module binary, meta.json, and first_run.sh into module.tar.gz -# so that `viam module reload-local` picks it up correctly (see meta.json build.path). STAGING_DIR="$(mktemp -d)" mkdir -p "${STAGING_DIR}/bin" cp build-conan/build/RelWithDebInfo/orbbec-module "${STAGING_DIR}/bin/" diff --git a/first_run.sh b/first_run.sh index 5ff2ee75..019e084f 100755 --- a/first_run.sh +++ b/first_run.sh @@ -5,8 +5,8 @@ set -euo pipefail OS=$(uname) if [[ "$OS" == 'Linux' ]]; then - cd $(dirname $0) + SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" # Install udev rules - sudo ./install_udev_rules.sh + sudo "$SCRIPT_DIR/install_udev_rules.sh" fi diff --git a/install_udev_rules.sh b/install_udev_rules.sh index 57ee62d1..271dab0d 100755 --- a/install_udev_rules.sh +++ b/install_udev_rules.sh @@ -11,7 +11,7 @@ CURR_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)" if [ "$(uname -s)" != "Darwin" ]; then # Install UDEV rules for USB device - cp ./99-obsensor-libusb.rules /etc/udev/rules.d/99-obsensor-libusb.rules + cp "$CURR_DIR/99-obsensor-libusb.rules" /etc/udev/rules.d/99-obsensor-libusb.rules echo "usb rules file install at /etc/udev/rules.d/99-obsensor-libusb.rules" fi udevadm control --reload-rules && udevadm trigger From 5b529d5908fd04a4ced9cdd3ca91848520b18b64 Mon Sep 17 00:00:00 2001 From: sean yu Date: Mon, 2 Mar 2026 17:49:25 -0500 Subject: [PATCH 5/9] refactor: use make module.tar.gz for build step instead of manual packaging 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. --- bin/build.sh | 8 -------- meta.json | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/bin/build.sh b/bin/build.sh index b2bcd62e..6dd406c0 100755 --- a/bin/build.sh +++ b/bin/build.sh @@ -39,11 +39,3 @@ conan build . \ -s:a "&:build_type=RelWithDebInfo" \ -s:a compiler.cppstd=17 -STAGING_DIR="$(mktemp -d)" -mkdir -p "${STAGING_DIR}/bin" -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}/" -tar czf module.tar.gz -C "${STAGING_DIR}" . -rm -rf "${STAGING_DIR}" diff --git a/meta.json b/meta.json index 93a25f38..c0e7980b 100644 --- a/meta.json +++ b/meta.json @@ -22,7 +22,7 @@ "first_run": "./first_run.sh", "build": { "setup": "bin/setup.sh", - "build": "make build", + "build": "make module.tar.gz", "path": "module.tar.gz", "arch": ["linux/amd64", "linux/arm64", "darwin/arm64", "windows/amd64"] } From ea3569792f264ef0c3888e06ec1198ae03bf4926 Mon Sep 17 00:00:00 2001 From: sean yu Date: Mon, 2 Mar 2026 07:47:47 -0500 Subject: [PATCH 6/9] =?UTF-8?q?RSDK-12844=20=20=20=20=E2=80=94=20=20=20=20?= =?UTF-8?q?Return=20errors=20from=20do=5Fcommand=20instead=20of=20empty=20?= =?UTF-8?q?struct?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/module/orbbec.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/module/orbbec.cpp b/src/module/orbbec.cpp index c4197b7f..bae67b2d 100644 --- a/src/module/orbbec.cpp +++ b/src/module/orbbec.cpp @@ -1486,7 +1486,8 @@ vsdk::ProtoStruct Orbbec::do_command(const vsdk::ProtoStruct& command) { const std::lock_guard lock(devices_by_serial_mu()); auto search = devices_by_serial().find(serial_number); if (search == devices_by_serial().end()) { - throw std::invalid_argument("device is not connected"); + VIAM_RESOURCE_LOG(error) << "[do_command] device is not connected"; + return vsdk::ProtoStruct{{"error", std::string("device is not connected")}}; } std::unique_ptr& dev = search->second; @@ -1575,6 +1576,9 @@ vsdk::ProtoStruct Orbbec::do_command(const vsdk::ProtoStruct& command) { return device_control::createModuleConfig(dev); } else if (key == "call_get_properties") { call_get_properties = true; + } else { + VIAM_RESOURCE_LOG(error) << "[do_command] unknown command: " << key; + return vsdk::ProtoStruct{{"error", "unknown command: " + key}}; } } } // unlock devices_by_serial_mu_ @@ -1603,6 +1607,7 @@ vsdk::ProtoStruct Orbbec::do_command(const vsdk::ProtoStruct& command) { } } catch (const std::exception& e) { VIAM_RESOURCE_LOG(error) << service_name << ": exception caught: " << e.what(); + return vsdk::ProtoStruct{{"error", std::string(e.what())}}; } return vsdk::ProtoStruct{}; } From ec1dd554858d58324985d4a429953fd937d7967a Mon Sep 17 00:00:00 2001 From: sean yu Date: Mon, 9 Mar 2026 13:01:28 -0400 Subject: [PATCH 7/9] Return error for unsupported property names in setDeviceProperty and setDeviceProperties Previously, when a user sent a property name that didn't match any supported property on the device, both setDeviceProperty and setDeviceProperties would silently return empty {} with no error. Fix setDeviceProperty to return an error dict when the property name is not found in the device's supported properties. Fix setDeviceProperties to check, after iterating supported properties, whether any user-supplied property names weren't found, and return an error for them. --- src/module/device_control.hpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/module/device_control.hpp b/src/module/device_control.hpp index d00813b9..486deb12 100644 --- a/src/module/device_control.hpp +++ b/src/module/device_control.hpp @@ -225,7 +225,7 @@ viam::sdk::ProtoStruct setDeviceProperty(std::shared_ptr device, } } - return {}; + return {{"error", "property not supported: " + property_map.begin()->first}}; } // Get property list @@ -299,6 +299,23 @@ viam::sdk::ProtoStruct setDeviceProperties(std::shared_ptr device, } } } + // Check for any user-supplied properties that weren't found in device's supported properties + for (auto const& [name, _] : properties_map) { + if (writable_properties.count(name) == 0) { + // Check if it's unsupported (not found) vs unwritable (handled above) + bool found = false; + for (int i = 0; i < supportedPropertyCount; i++) { + OBPropertyItem property_item = device->getSupportedProperty(i); + if (property_item.name == name) { + found = true; + break; + } + } + if (!found) { + return {{"error", "property not supported: " + name}}; + } + } + } return getDeviceProperties(device, command, writable_properties); } From a37301e8ef1e4c260b97aaaa3896fbe1a48a4939 Mon Sep 17 00:00:00 2001 From: sean yu Date: Mon, 9 Mar 2026 13:10:34 -0400 Subject: [PATCH 8/9] return error for empty do_command --- src/module/orbbec.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/module/orbbec.cpp b/src/module/orbbec.cpp index e36b7408..36987031 100644 --- a/src/module/orbbec.cpp +++ b/src/module/orbbec.cpp @@ -1471,6 +1471,9 @@ vsdk::Camera::image_collection Orbbec::get_images(std::vector filte } vsdk::ProtoStruct Orbbec::do_command(const vsdk::ProtoStruct& command) { + if (command.empty()) { + return vsdk::ProtoStruct{{"error", std::string("empty command")}}; + } bool call_get_properties = false; try { constexpr char firmware_key[] = "update_firmware"; From c0a70b40c9c12da389ac5bc276b895694c561045 Mon Sep 17 00:00:00 2001 From: sean yu Date: Mon, 9 Mar 2026 13:12:24 -0400 Subject: [PATCH 9/9] simplify unsupported property check in setDeviceProperties --- src/module/device_control.hpp | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/module/device_control.hpp b/src/module/device_control.hpp index 486deb12..37f48a82 100644 --- a/src/module/device_control.hpp +++ b/src/module/device_control.hpp @@ -253,11 +253,13 @@ viam::sdk::ProtoStruct setDeviceProperties(std::shared_ptr device, return {{"error", "properties must be a struct"}}; } std::unordered_set writable_properties; + std::unordered_set seen_properties; auto const& properties_map = properties.template get_unchecked(); int const supportedPropertyCount = device->getSupportedPropertyCount(); for (int i = 0; i < supportedPropertyCount; i++) { OBPropertyItem property_item = device->getSupportedProperty(i); if (properties_map.count(property_item.name) > 0) { + seen_properties.insert(property_item.name); if (property_item.permission == OB_PERMISSION_DENY || property_item.permission == OB_PERMISSION_READ) { std::stringstream error_ss; error_ss << "Property " << property_item.name << " is not writable, skipping."; @@ -299,21 +301,9 @@ viam::sdk::ProtoStruct setDeviceProperties(std::shared_ptr device, } } } - // Check for any user-supplied properties that weren't found in device's supported properties for (auto const& [name, _] : properties_map) { - if (writable_properties.count(name) == 0) { - // Check if it's unsupported (not found) vs unwritable (handled above) - bool found = false; - for (int i = 0; i < supportedPropertyCount; i++) { - OBPropertyItem property_item = device->getSupportedProperty(i); - if (property_item.name == name) { - found = true; - break; - } - } - if (!found) { - return {{"error", "property not supported: " + name}}; - } + if (seen_properties.count(name) == 0) { + return {{"error", "property not supported: " + name}}; } } return getDeviceProperties(device, command, writable_properties);