Bump viam-cpp-sdk to 0.38.1 and drop Reconfigurable

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.
 #

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
 #

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")

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 \

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

src/module/discovery.hpp

@@ -2,7 +2,6 @@
 #include <memory>
 
 #include <viam/sdk/config/resource.hpp>
-#include <viam/sdk/resource/reconfigurable.hpp>
 #include <viam/sdk/services/discovery.hpp>
 
 #include <libobsensor/ObSensor.hpp>
@@ -16,6 +15,7 @@ class OrbbecDiscovery : public viam::sdk::Discovery {
                              std::shared_ptr<ob::Context> ctx);
     std::vector<viam::sdk::ResourceConfig> 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:

src/module/orbbec.cpp

@@ -38,7 +38,6 @@
 #include <viam/sdk/config/resource.hpp>
 #include <viam/sdk/module/service.hpp>
 #include <viam/sdk/registry/registry.hpp>
-#include <viam/sdk/resource/reconfigurable.hpp>
 #include <viam/sdk/rpc/server.hpp>
 
 #include <libobsensor/ObSensor.hpp>
@@ -1193,124 +1192,44 @@ 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<std::mutex> lock_serial(serial_number_mu_);
-        const std::lock_guard<std::mutex> 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;
+    // 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.
     {
-        auto config = configure(deps, cfg);
-        {
-            const std::lock_guard<std::mutex> 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<std::mutex> 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();
+        const std::lock_guard<std::mutex> lock(frame_set_by_serial_mu());
+        frame_set_by_serial().erase(serial_number_);
     }
 
+    VIAM_RESOURCE_LOG(info) << "Orbbec destructor end " << serial_number_;
+}
+
+vsdk::ProtoStruct Orbbec::get_status() {
+    std::string serial_number;
     {
-        std::lock_guard<std::mutex> lock(frame_set_by_serial_mu());
-        frame_set_by_serial().erase(prev_serial_number);
+        const std::lock_guard<std::mutex> lock(serial_number_mu_);
+        serial_number = serial_number_;
     }
 
-    // set firmware version member variable and apply experimental config
+    bool started = false;
     {
-        std::lock_guard<std::mutex> lock_serial(serial_number_mu_);
-        std::lock_guard<std::mutex> lock(devices_by_serial_mu());
-        auto search = devices_by_serial().find(serial_number_);
+        const std::lock_guard<std::mutex> 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<ob::DeviceInfo> deviceInfo = search->second->device->getDeviceInfo();
-            model_config_ = OrbbecModelConfig::forDevice(deviceInfo->name());
-
-            std::unique_ptr<ViamOBDevice>& my_dev = search->second;
-            applyExperimentalConfig(my_dev, cfg.attributes());
+            started = search->second->started;
         }
     }
 
-    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<std::mutex> lock(serial_by_resource_mu());
-        serial_by_resource()[new_resource_name] = new_serial_number;
-    }
-    VIAM_RESOURCE_LOG(info) << "[reconfigure] Orbbec reconfigure end";
-}
-
-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<std::mutex> 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<ob::FrameSet> fs = nullptr;
-        {
-            std::lock_guard<std::mutex> 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<ob::Frame> color = fs->getFrame(OB_FRAME_COLOR);
-
-        std::optional<DeviceFormat> res_format_opt;
-        {
-            std::lock_guard<std::mutex> 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;
-        }
-
-        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() {
@@ -1730,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();
 

src/module/orbbec.hpp

@@ -2,7 +2,6 @@
 #include <viam/sdk/common/pose.hpp>
 #include <viam/sdk/components/camera.hpp>
 #include <viam/sdk/config/resource.hpp>
-#include <viam/sdk/resource/reconfigurable.hpp>
 #include <viam/sdk/spatialmath/geometry.hpp>
 
 #include <libobsensor/ObSensor.hpp>
@@ -156,13 +155,16 @@ struct ViamOBDevice {
 void startOrbbecSDK(ob::Context& ctx);
 void printDeviceInfo(const std::shared_ptr<ob::DeviceInfo> 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<ob::Context> 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;
+    viam::sdk::ProtoStruct get_status() override;
     image_collection get_images(std::vector<std::string> 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;

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
 				}
 			}

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 (

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=