camera: usb: Still enable USB HAL when CrOS config fails
CrOS config may fail because the device identity (sku id, firmware
version, etc) is mis-configured in firmware or config files. Do not
block USB camera basic functionality in this case.
The CrosDeviceConfig is refactored to avoid accessing uninitialized
data.
BUG=b:174197592,b:169781914
TEST=Deploy and check USB HAL initialization.
Change-Id: I4f79a34e9fe5e84c96c7196c27d26552ec33abb5
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2562990
Commit-Queue: Ren-Pei Zeng <kamesan@chromium.org>
Tested-by: Ren-Pei Zeng <kamesan@chromium.org>
Reviewed-by: Shik Chen <shik@chromium.org>
diff --git a/camera/hal/usb/camera_hal.cc b/camera/hal/usb/camera_hal.cc
index 47a4492..df0116b 100644
--- a/camera/hal/usb/camera_hal.cc
+++ b/camera/hal/usb/camera_hal.cc
@@ -111,7 +111,7 @@
CameraHal::CameraHal()
: task_runner_(nullptr),
udev_watcher_(std::make_unique<UdevWatcher>(this, "video4linux")),
- cros_device_config_(CrosDeviceConfig::Get()) {
+ cros_device_config_(CrosDeviceConfig::Create()) {
thread_checker_.DetachFromThread();
}
@@ -146,9 +146,11 @@
LOGF(ERROR) << "Camera " << id << " is already opened";
return -EBUSY;
}
- if (!cameras_.empty() && (cros_device_config_.model_name == "treeya360" ||
- cros_device_config_.model_name == "nuwani360" ||
- cros_device_config_.model_name == "pompom")) {
+ if (!cameras_.empty() &&
+ (cros_device_config_ != nullptr &&
+ (cros_device_config_->GetModelName() == "treeya360" ||
+ cros_device_config_->GetModelName() == "nuwani360" ||
+ cros_device_config_->GetModelName() == "pompom"))) {
// It cannot open multiple cameras at the same time due to USB bandwidth
// limitation (b/147333530, b/171856355).
// TODO(shik): Use |conflicting_devices| to implement this logic after we
@@ -227,9 +229,9 @@
int CameraHal::Init() {
DCHECK(thread_checker_.CalledOnValidThread());
- if (!cros_device_config_.is_initialized) {
- LOGF(ERROR) << "Failed to initialize CrOS device config";
- return -ENODEV;
+ if (cros_device_config_ == nullptr) {
+ LOGF(WARNING) << "Failed to initialize CrOS device config, camera HAL may "
+ "function incorrectly";
}
if (!udev_watcher_->Start(base::ThreadTaskRunnerHandle::Get())) {
@@ -270,9 +272,10 @@
num_builtin_cameras_ = 1;
}
- if (cros_device_config_.usb_camera_count.has_value()) {
- if (num_builtin_cameras_ != *cros_device_config_.usb_camera_count) {
- LOGF(ERROR) << "Expected " << *cros_device_config_.usb_camera_count
+ if (cros_device_config_ != nullptr &&
+ cros_device_config_->IsUsbCameraCountAvailable()) {
+ if (num_builtin_cameras_ != cros_device_config_->GetUsbCameraCount()) {
+ LOGF(ERROR) << "Expected " << cros_device_config_->GetUsbCameraCount()
<< " cameras from Chrome OS config, found "
<< num_builtin_cameras_;
return -ENODEV;
@@ -429,7 +432,8 @@
// Mark the camera as v1 if it is a built-in camera and the CrOS device is
// marked as a v1 device.
- if (info_ptr != nullptr && cros_device_config_.is_v1_device) {
+ if (info_ptr != nullptr && cros_device_config_ != nullptr &&
+ cros_device_config_->IsV1Device()) {
info.quirks |= kQuirkV1Device;
}
diff --git a/camera/hal/usb/camera_hal.h b/camera/hal/usb/camera_hal.h
index c108bc0..3ccf7ae 100644
--- a/camera/hal/usb/camera_hal.h
+++ b/camera/hal/usb/camera_hal.h
@@ -100,7 +100,7 @@
std::unique_ptr<UdevWatcher> udev_watcher_;
// Used to access to the main configuration for Chrome OS.
- CrosDeviceConfig cros_device_config_;
+ std::unique_ptr<CrosDeviceConfig> cros_device_config_;
// Map from device path to camera id.
std::map<std::string, int> path_to_id_;
diff --git a/camera/hal/usb/cros_device_config.cc b/camera/hal/usb/cros_device_config.cc
index 932acd2..093b7ee 100644
--- a/camera/hal/usb/cros_device_config.cc
+++ b/camera/hal/usb/cros_device_config.cc
@@ -20,22 +20,18 @@
} // namespace
-CrosDeviceConfig::CrosDeviceConfig() {}
-
-CrosDeviceConfig::~CrosDeviceConfig() {}
-
-CrosDeviceConfig CrosDeviceConfig::Get() {
+std::unique_ptr<CrosDeviceConfig> CrosDeviceConfig::Create() {
CrosDeviceConfig res = {};
brillo::CrosConfig cros_config;
if (!cros_config.Init()) {
LOGF(ERROR) << "Failed to initialize CrOS config";
- return res;
+ return nullptr;
}
if (!cros_config.GetString("/", "name", &res.model_name)) {
LOGF(ERROR) << "Failed to get model name of CrOS device";
- return res;
+ return nullptr;
}
std::string use_legacy_usb;
@@ -80,8 +76,7 @@
return count;
}();
- res.is_initialized = true;
- return res;
+ return std::make_unique<CrosDeviceConfig>(res);
}
} // namespace cros
diff --git a/camera/hal/usb/cros_device_config.h b/camera/hal/usb/cros_device_config.h
index 52452a7..1f6390b 100644
--- a/camera/hal/usb/cros_device_config.h
+++ b/camera/hal/usb/cros_device_config.h
@@ -7,6 +7,7 @@
#ifndef CAMERA_HAL_USB_CROS_DEVICE_CONFIG_H_
#define CAMERA_HAL_USB_CROS_DEVICE_CONFIG_H_
+#include <memory>
#include <string>
#include <base/optional.h>
@@ -15,16 +16,20 @@
namespace cros {
// This structs wraps the brillo::CrosConfig and stores the required values.
-struct CrosDeviceConfig {
+class CrosDeviceConfig {
public:
- CrosDeviceConfig();
- CrosDeviceConfig(const CrosDeviceConfig& other) = default;
- CrosDeviceConfig& operator=(const CrosDeviceConfig& other) = default;
- ~CrosDeviceConfig();
+ static std::unique_ptr<CrosDeviceConfig> Create();
- static CrosDeviceConfig Get();
+ bool IsV1Device() const { return is_v1_device; }
+ const std::string& GetModelName() const { return model_name; }
+ bool IsUsbCameraCountAvailable() const {
+ return usb_camera_count.has_value();
+ }
+ int GetUsbCameraCount() const { return *usb_camera_count; }
- bool is_initialized;
+ private:
+ CrosDeviceConfig() = default;
+
bool is_v1_device;
std::string model_name;
base::Optional<int> usb_camera_count;
