From 66dc3f8a6856ac8fa7c4549359026a244124caca Mon Sep 17 00:00:00 2001 From: overwatch-bot Date: Wed, 22 Jul 2026 10:15:43 +0000 Subject: [PATCH 1/4] chore(deps): bump Viam SDKs --- bin/setup.ps1 | 2 +- bin/setup.sh | 2 +- conanfile.py | 2 +- etc/Dockerfile.ubuntu.jammy | 2 +- tests/go.mod | 4 ++-- tests/go.sum | 8 ++++---- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/bin/setup.ps1 b/bin/setup.ps1 index b34ea337..5230ca46 100644 --- a/bin/setup.ps1 +++ b/bin/setup.ps1 @@ -45,7 +45,7 @@ Push-Location viam-cpp-sdk # NOTE: If you change this version, also change it in the `conanfile.py` requirements # and in dockerfile -git checkout releases/v0.20.1 +git checkout releases/v0.38.1 # Build the C++ SDK repo. # diff --git a/bin/setup.sh b/bin/setup.sh index 6457a1ae..04f1bb37 100755 --- a/bin/setup.sh +++ b/bin/setup.sh @@ -55,7 +55,7 @@ fi # NOTE: If you change this version, also change it in the `conanfile.py` requirements # and in the Dockerfile -git checkout releases/v0.20.1 +git checkout releases/v0.38.1 # Build the C++ SDK repo # diff --git a/conanfile.py b/conanfile.py index df16d499..bbd3f1ad 100644 --- a/conanfile.py +++ b/conanfile.py @@ -28,7 +28,7 @@ def set_version(self): def requirements(self): # NOTE: If you update the `viam-cpp-sdk` dependency here, it # should also be updated in `bin/setup.{sh,ps1}`, and in the Dockerfile. - self.requires("viam-cpp-sdk/0.20.1") + self.requires("viam-cpp-sdk/0.38.1") self.requires("openssl/[>=3 <4]") self.requires("libcurl/8.9.1") self.requires("libzip/1.11.1") diff --git a/etc/Dockerfile.ubuntu.jammy b/etc/Dockerfile.ubuntu.jammy index 3684281f..5843dff5 100644 --- a/etc/Dockerfile.ubuntu.jammy +++ b/etc/Dockerfile.ubuntu.jammy @@ -50,7 +50,7 @@ RUN conan profile detect # NOTE: If you update the `viam-cpp-sdk` dependency here, it # should also be updated in `bin/setup.{sh,ps1}`, and in the conanfile.py. RUN git clone https://github.com/viamrobotics/viam-cpp-sdk.git \ - --branch releases/v0.19.0 --depth=1 && \ + --branch releases/v0.38.1 --depth=1 && \ cd viam-cpp-sdk && \ conan create . \ --build=missing \ diff --git a/tests/go.mod b/tests/go.mod index f91d39f8..dd2a525f 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -5,8 +5,8 @@ go 1.25.9 require ( github.com/golang/geo v0.0.0-20230421003525-6adc56603217 go.viam.com/rdk v1.0.0 - go.viam.com/test v1.2.4 - go.viam.com/utils v0.6.6 + go.viam.com/test v1.2.5 + go.viam.com/utils v0.8.0 ) require ( diff --git a/tests/go.sum b/tests/go.sum index 4abdf94d..12a6c211 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -922,10 +922,10 @@ go.viam.com/api v0.1.566 h1:o5nWDj6at04Y8nHGC7mZQDA6NIuGdPpwqhNLY+LQtfs= go.viam.com/api v0.1.566/go.mod h1:nVe4WXrtc8aupJ8OWXSYx6KhCiOkr3VCbkwxD4D41xQ= go.viam.com/rdk v1.0.0 h1:i7nTVaccN7N045xwNyYm3vBspuY+NzwT4PX9vLw7GxU= go.viam.com/rdk v1.0.0/go.mod h1:1dl6guxr5j+Th2CsQNcIF5uNgVaFuIiKHZm1Cu5G7Bo= -go.viam.com/test v1.2.4 h1:JYgZhsuGAQ8sL9jWkziAXN9VJJiKbjoi9BsO33TW3ug= -go.viam.com/test v1.2.4/go.mod h1:zI2xzosHdqXAJ/kFqcN+OIF78kQuTV2nIhGZ8EzvaJI= -go.viam.com/utils v0.6.6 h1:1R+SpKz1McCAIC0JshgkfHy+uDu4okNmDq/AYl9m8iE= -go.viam.com/utils v0.6.6/go.mod h1:sAqzMj1M4weq/0HIqdfzrjCZ6zIzDhUemH8Tml1Hyag= +go.viam.com/test v1.2.5 h1:k2bdMTlINKOmwp6BbZTCEu4X0VDsx7h0ePQObvFBI8A= +go.viam.com/test v1.2.5/go.mod h1:t7S4N0LRX8DukUWldF68zunZ6fPLuZMEFPjvDWi/WV0= +go.viam.com/utils v0.8.0 h1:urXO5RoVahTLnsa4DaH46bF+5Ec9D47vfwEbilZ+/gI= +go.viam.com/utils v0.8.0/go.mod h1:sAqzMj1M4weq/0HIqdfzrjCZ6zIzDhUemH8Tml1Hyag= go4.org/unsafe/assume-no-moving-gc v0.0.0-20230525183740-e7c30c78aeb2 h1:WJhcL4p+YeDxmZWg141nRm7XC8IDmhz7lk5GpadO1Sg= go4.org/unsafe/assume-no-moving-gc v0.0.0-20230525183740-e7c30c78aeb2/go.mod h1:FftLjUGFEDu5k8lt0ddY+HcrH/qU/0qk+H8j9/nTl3E= goji.io v2.0.2+incompatible h1:uIssv/elbKRLznFUy3Xj4+2Mz/qKhek/9aZQDUMae7c= From 2ddf459dbfac68721b3d81d2bcdf553255580a96 Mon Sep 17 00:00:00 2001 From: Nicolas Palpacuer Date: Tue, 28 Jul 2026 14:51:53 -0400 Subject: [PATCH 2/4] Drop Reconfigurable, moving its cleanup into the destructor viam-cpp-sdk removed viam::sdk::Reconfigurable in v0.35.0 (viamrobotics/viam-cpp-sdk#630). viam-server now rebuilds a resource on a config change instead of calling reconfigure, so the interface, the override, and the three includes of reconfigurable.hpp all go away. The include in discovery.hpp was already unused. The ordering is safe: ModuleService::ReconfigureResource calls ResourceManager::replace_one, which runs do_remove(name) before do_add(name, create_resource()), so the old instance is destructed before the replacement is constructed. The constructor already redoes everything reconfigure did -- configure, configureDevice, startDevice, and the serial_by_resource mapping. Two cleanups only reconfigure performed do not survive that on their own, so move them into ~Orbbec(): config_by_serial().erase(...) otherwise a stale entry leaks whenever the serial_number attribute changes frame_set_by_serial().erase(...) otherwise the rebuilt instance can find a frame from the previous configuration and report "no recent frame: check connection" The second is a regression risk rather than hygiene: it was added by 77d353c ("RSDK-11302 bug fix reconfigure causes no recent frame error") precisely to stop that error, so deleting reconfigure without moving it would reopen that bug. The erase runs after stopDevice so the pipeline is already stopped and no further frames can land. Co-Authored-By: Claude Opus 5 (1M context) --- src/module/discovery.hpp | 1 - src/module/orbbec.cpp | 78 +++++----------------------------------- src/module/orbbec.hpp | 8 +++-- 3 files changed, 14 insertions(+), 73 deletions(-) diff --git a/src/module/discovery.hpp b/src/module/discovery.hpp index 17df84e2..6312c114 100644 --- a/src/module/discovery.hpp +++ b/src/module/discovery.hpp @@ -2,7 +2,6 @@ #include #include -#include #include #include diff --git a/src/module/orbbec.cpp b/src/module/orbbec.cpp index a24fce2c..0c122d17 100644 --- a/src/module/orbbec.cpp +++ b/src/module/orbbec.cpp @@ -38,7 +38,6 @@ #include #include #include -#include #include #include @@ -1193,81 +1192,22 @@ Orbbec::~Orbbec() { } else { prev_serial_number = config_by_serial().at(serial_number_).serial_number; prev_resource_name = config_by_serial().at(serial_number_).resource_name; + config_by_serial().erase(serial_number_); } } stopDevice(prev_serial_number, prev_resource_name); - VIAM_RESOURCE_LOG(info) << "Orbbec destructor end " << serial_number_; -} - -void Orbbec::reconfigure(const vsdk::Dependencies& deps, const vsdk::ResourceConfig& cfg) { - VIAM_RESOURCE_LOG(info) << "[reconfigure] Orbbec reconfigure start"; - std::string prev_serial_number; - std::string prev_resource_name; - { - const std::lock_guard lock_serial(serial_number_mu_); - const std::lock_guard lock(config_by_serial_mu()); - if (config_by_serial().count(serial_number_) == 0) { - std::ostringstream buffer; - buffer << "[reconfigure] device with serial number " << serial_number_ << " is not in config_by_serial, skipping reconfigure"; - VIAM_RESOURCE_LOG(error) << buffer.str(); - throw std::runtime_error(buffer.str()); - } else { - prev_serial_number = config_by_serial().at(serial_number_).serial_number; - prev_resource_name = config_by_serial().at(serial_number_).resource_name; - } - } - stopDevice(prev_serial_number, prev_resource_name); - std::string new_serial_number; - std::string new_resource_name; - { - auto config = configure(deps, cfg); - { - const std::lock_guard lock(config_by_serial_mu()); - config_by_serial().erase(prev_serial_number); - config_by_serial().insert_or_assign(config->serial_number, *config); - } - { - const std::lock_guard lock(serial_number_mu_); - serial_number_ = config->serial_number; - } - new_serial_number = config->serial_number; - new_resource_name = config->resource_name; - VIAM_RESOURCE_LOG(info) << "[reconfigure] updated config_by_serial_: " << config_by_serial().at(new_serial_number).to_string(); - } + // Drop any frame captured before this teardown. viam-server rebuilds the resource on a config + // change, so leaving one here lets the replacement instance find a frame from the previous + // configuration: the staleness check then reports "no recent frame: check connection" rather + // than the frames simply not having arrived yet (RSDK-11302). This has to run after stopDevice, + // which stops the pipeline, so that no further frames can land after the erase. { - std::lock_guard lock(frame_set_by_serial_mu()); - frame_set_by_serial().erase(prev_serial_number); - } - - // set firmware version member variable and apply experimental config - { - std::lock_guard lock_serial(serial_number_mu_); - std::lock_guard lock(devices_by_serial_mu()); - auto search = devices_by_serial().find(serial_number_); - if (search != devices_by_serial().end()) { - firmware_version_ = search->second->device->getDeviceInfo()->firmwareVersion(); - - // Detect model and set model_config_ - std::shared_ptr deviceInfo = search->second->device->getDeviceInfo(); - model_config_ = OrbbecModelConfig::forDevice(deviceInfo->name()); - - std::unique_ptr& my_dev = search->second; - applyExperimentalConfig(my_dev, cfg.attributes()); - } + const std::lock_guard lock(frame_set_by_serial_mu()); + frame_set_by_serial().erase(serial_number_); } - if (!model_config_.has_value()) { - throw std::runtime_error("Failed to detect Orbbec model configuration during reconfigure"); - } - - configureDevice(new_serial_number, model_config_.value()); - startDevice(new_serial_number, model_config_.value()); - { - std::lock_guard lock(serial_by_resource_mu()); - serial_by_resource()[new_resource_name] = new_serial_number; - } - VIAM_RESOURCE_LOG(info) << "[reconfigure] Orbbec reconfigure end"; + VIAM_RESOURCE_LOG(info) << "Orbbec destructor end " << serial_number_; } vsdk::Camera::raw_image Orbbec::get_image(std::string mime_type, const vsdk::ProtoStruct& extra) { diff --git a/src/module/orbbec.hpp b/src/module/orbbec.hpp index 9ce3fb14..ddf0d117 100644 --- a/src/module/orbbec.hpp +++ b/src/module/orbbec.hpp @@ -2,7 +2,6 @@ #include #include #include -#include #include #include @@ -156,11 +155,14 @@ struct ViamOBDevice { void startOrbbecSDK(ob::Context& ctx); void printDeviceInfo(const std::shared_ptr info); -class Orbbec final : public viam::sdk::Camera, public viam::sdk::Reconfigurable { +// viam-server rebuilds the resource on a config change rather than calling a reconfigure method, +// so everything a reconfigure would have done lives in the constructor and destructor. The SDK +// destroys the old instance before constructing the replacement, so the destructor must leave the +// device in a state the constructor can start from. +class Orbbec final : public viam::sdk::Camera { public: Orbbec(viam::sdk::Dependencies deps, viam::sdk::ResourceConfig cfg, std::shared_ptr ctx); ~Orbbec(); - void reconfigure(const viam::sdk::Dependencies& deps, const viam::sdk::ResourceConfig& cfg) override; viam::sdk::ProtoStruct do_command(const viam::sdk::ProtoStruct& command) override; raw_image get_image(std::string mime_type, const viam::sdk::ProtoStruct& extra) override; image_collection get_images(std::vector filter_source_names, const viam::sdk::ProtoStruct& extra) override; From 2cb0d4a4302b3df128d2d0bafaab027e412c57d1 Mon Sep 17 00:00:00 2001 From: Nicolas Palpacuer Date: Tue, 28 Jul 2026 16:33:11 -0400 Subject: [PATCH 3/4] Implement get_status and drop get_image for the v0.38.1 Camera API Two more interface changes surfaced once the reconfigurable.hpp fatal include error stopped masking them. get_image is gone from the Camera interface, and CameraServer no longer serves a GetImage RPC at all -- its handlers are DoCommand, GetImages, GetPointCloud, GetGeometries, GetProperties and GetStatus. Nothing could reach our implementation any more and nothing called it internally, so remove it rather than keep it as an unreachable method. get_images already covers the color stream and has since #45. get_status is a new pure virtual on both Camera and Discovery, which is what made Orbbec and OrbbecDiscovery abstract: error: invalid new-expression of abstract class type 'orbbec::Orbbec' error: invalid new-expression of abstract class type 'discovery::OrbbecDiscovery' The SDK prescribes no schema -- its own mocks return an arbitrary struct -- so Orbbec reports the identifying state it already tracks: serial number, detected model, firmware version, and whether the pipeline is streaming. OrbbecDiscovery has no state of its own and returns an empty struct rather than enumerating the bus on every status poll. Also corrects a log label in get_point_cloud that read "[get_image]", which now names a function that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) --- src/module/discovery.cpp | 7 +++++ src/module/discovery.hpp | 1 + src/module/orbbec.cpp | 57 +++++++++++++--------------------------- src/module/orbbec.hpp | 2 +- 4 files changed, 27 insertions(+), 40 deletions(-) diff --git a/src/module/discovery.cpp b/src/module/discovery.cpp index b149e350..2d89df82 100644 --- a/src/module/discovery.cpp +++ b/src/module/discovery.cpp @@ -100,4 +100,11 @@ vsdk::ProtoStruct OrbbecDiscovery::do_command(const vsdk::ProtoStruct& command) return vsdk::ProtoStruct{}; } +// The service holds no state of its own: what is attached is reported by discover_resources, which +// enumerates the bus on demand. Deliberately not querying the device list here, so that polling +// status stays cheap and cannot disturb an in-progress enumeration. +vsdk::ProtoStruct OrbbecDiscovery::get_status() { + return vsdk::ProtoStruct{}; +} + } // namespace discovery diff --git a/src/module/discovery.hpp b/src/module/discovery.hpp index 6312c114..c77f5423 100644 --- a/src/module/discovery.hpp +++ b/src/module/discovery.hpp @@ -15,6 +15,7 @@ class OrbbecDiscovery : public viam::sdk::Discovery { std::shared_ptr ctx); std::vector discover_resources(const viam::sdk::ProtoStruct& extra) override; viam::sdk::ProtoStruct do_command(const viam::sdk::ProtoStruct& command) override; + viam::sdk::ProtoStruct get_status() override; static viam::sdk::Model model; private: diff --git a/src/module/orbbec.cpp b/src/module/orbbec.cpp index 0c122d17..dbf0ed8a 100644 --- a/src/module/orbbec.cpp +++ b/src/module/orbbec.cpp @@ -1210,47 +1210,26 @@ Orbbec::~Orbbec() { VIAM_RESOURCE_LOG(info) << "Orbbec destructor end " << serial_number_; } -vsdk::Camera::raw_image Orbbec::get_image(std::string mime_type, const vsdk::ProtoStruct& extra) { - try { - VIAM_RESOURCE_LOG(debug) << "[get_image] start"; - std::string serial_number; - { - const std::lock_guard lock(serial_number_mu_); - serial_number = serial_number_; - } - - if (model_config_.has_value()) { - checkFirmwareVersion(firmware_version_, model_config_->min_firmware_version, model_config_->viam_model_suffix); - } - - std::shared_ptr fs = nullptr; - { - std::lock_guard lock(frame_set_by_serial_mu()); - auto search = frame_set_by_serial().find(serial_number); - if (search == frame_set_by_serial().end()) { - throw std::invalid_argument("no frame yet"); - } - fs = search->second; - } - std::shared_ptr color = fs->getFrame(OB_FRAME_COLOR); +vsdk::ProtoStruct Orbbec::get_status() { + std::string serial_number; + { + const std::lock_guard lock(serial_number_mu_); + serial_number = serial_number_; + } - std::optional res_format_opt; - { - std::lock_guard lock(config_by_serial_mu()); - if (config_by_serial().count(serial_number) == 0) { - throw std::invalid_argument("device with serial number " + serial_number + " is not in config_by_serial"); - } - res_format_opt = config_by_serial().at(serial_number).device_format; + bool started = false; + { + const std::lock_guard lock(devices_by_serial_mu()); + auto search = devices_by_serial().find(serial_number); + if (search != devices_by_serial().end()) { + started = search->second->started; } - - validateColorFrame(color, res_format_opt, *model_config_); - vsdk::Camera::raw_image response = encodeColorFrame(color); - VIAM_RESOURCE_LOG(debug) << "[get_image] end"; - return response; - } catch (const std::exception& e) { - VIAM_RESOURCE_LOG(error) << "[get_image] error: " << e.what(); - throw std::runtime_error("failed to create image: " + std::string(e.what())); } + + return vsdk::ProtoStruct{{"serial_number", serial_number}, + {"model", model_config_.has_value() ? model_config_->viam_model_suffix : std::string{}}, + {"firmware_version", firmware_version_}, + {"streaming", started}}; } vsdk::Camera::properties Orbbec::get_properties() { @@ -1670,7 +1649,7 @@ vsdk::Camera::point_cloud Orbbec::get_point_cloud(std::string mime_type, const v std::uint8_t* colorData = (std::uint8_t*)color->getData(); if (colorData == nullptr) { - throw std::runtime_error("[get_image] color data is null"); + throw std::runtime_error("[get_point_cloud] color data is null"); } std::uint32_t colorDataSize = color->dataSize(); diff --git a/src/module/orbbec.hpp b/src/module/orbbec.hpp index ddf0d117..75d859fe 100644 --- a/src/module/orbbec.hpp +++ b/src/module/orbbec.hpp @@ -164,7 +164,7 @@ class Orbbec final : public viam::sdk::Camera { Orbbec(viam::sdk::Dependencies deps, viam::sdk::ResourceConfig cfg, std::shared_ptr ctx); ~Orbbec(); viam::sdk::ProtoStruct do_command(const viam::sdk::ProtoStruct& command) override; - raw_image get_image(std::string mime_type, const viam::sdk::ProtoStruct& extra) override; + viam::sdk::ProtoStruct get_status() override; image_collection get_images(std::vector filter_source_names, const viam::sdk::ProtoStruct& extra) override; point_cloud get_point_cloud(std::string mime_type, const viam::sdk::ProtoStruct& extra) override; properties get_properties() override; From 629de8aba6c5f93bf6416695a62ea49305267fd6 Mon Sep 17 00:00:00 2001 From: Nicolas Palpacuer Date: Tue, 28 Jul 2026 17:11:49 -0400 Subject: [PATCH 4/4] Move single-image test onto Images() with a source filter viam-cpp-sdk removed Camera::get_image and its CameraServer no longer serves a GetImage RPC, so DecodeImageFromCamera has nothing to reach on this module. Ask Images() for just the color source instead, which keeps the test's intent -- retrieve one image and check it is the colour stream -- and additionally covers filter_source_names, which nothing exercised before. Source names are "color" and "depth", as documented in the README. Co-Authored-By: Claude Opus 5 (1M context) --- tests/astra2_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/astra2_test.go b/tests/astra2_test.go index e9c50c0b..359ba621 100644 --- a/tests/astra2_test.go +++ b/tests/astra2_test.go @@ -66,19 +66,23 @@ func TestCameraServer(t *testing.T) { } defer cam.Close(timeoutCtx) - t.Run("Get image method (one image)", func(t *testing.T) { + // Single-source retrieval goes through Images with a filter rather than the image API: + // viam-cpp-sdk removed Camera::get_image and the CameraServer no longer serves a GetImage + // RPC at all, so there is nothing behind DecodeImageFromCamera for this module to answer. + t.Run("Get images method (color only)", func(t *testing.T) { timeout := time.After(testTimeoutDuration) tick := time.Tick(testTickDuration) for { select { case <-timeout: - t.Fatal("timed out waiting for Get image method (one image)") + t.Fatal("timed out waiting for Get images method (color only)") case <-tick: - img, err := camera.DecodeImageFromCamera(timeoutCtx, cam, nil, nil) - if err != nil { + images, _, err := cam.Images(timeoutCtx, []string{"color"}, nil) + if err != nil || len(images) < 1 { continue } - test.That(t, img, test.ShouldNotBeNil) + test.That(t, len(images), test.ShouldEqual, 1) + test.That(t, images[0].SourceName, test.ShouldEqual, "color") return } }