Decide which DLCs to install in s2

The DLC IDs are changed to from enum to strings to allow parsing
from board.ini without requiring listing them all in the source code.

FIXED=b:356757315
TEST=CQ

Change-Id: I2bf4767df124fa2662951a3e0db3a03829b097c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/6040975
Commit-Queue: Li-Yu Yu <aaronyu@google.com>
Tested-by: Li-Yu Yu <aaronyu@google.com>
Reviewed-by: Hung-Hsien Chen <hunghsienchen@chromium.org>

Cargo.Bazel.lock

@@ -1,5 +1,5 @@
 {
-  "checksum": "9a241194a05459dda7f4c65786ed4ee920464ebcd778b6388ef33fdb661c390a",
+  "checksum": "3617ba629cc3c6bf6b4df0b7596a6285147b1f712bf925e4ffd13e4f59dc3d4b",
   "crates": {
     "addr2line 0.20.0": {
       "name": "addr2line",
@@ -2130,6 +2130,10 @@
             {
               "id": "thiserror 1.0.50",
               "target": "thiserror"
+            },
+            {
+              "id": "zerocopy 0.7.34",
+              "target": "zerocopy"
             }
           ],
           "selects": {}

cras/common/rust_common.h

@@ -18,19 +18,6 @@ extern "C" {
 #include <stdint.h>
 #include <stdlib.h>
 
-#define NUM_CRAS_DLCS 3
-
-#define CRAS_DLC_ID_STRING_MAX_LENGTH 50
-
-/**
- * All supported DLCs in CRAS.
- */
-enum CrasDlcId {
-  CrasDlcSrBt,
-  CrasDlcNcAp,
-  CrasDlcIntelligoBeamforming,
-};
-
 enum CRAS_FRA_SIGNAL {
   PeripheralsUsbSoundCard = 0,
   USBAudioConfigureFailed,

cras/common/src/types_internal.rs

@@ -5,9 +5,7 @@
 use std::borrow::Cow;
 use std::ffi::CStr;
 use std::ffi::CString;
-use std::fmt::Display;
 
-use anyhow::bail;
 use bitflags::bitflags;
 use itertools::Itertools;
 use serde::Serialize;
@@ -184,59 +182,8 @@ pub struct CrasEffectUIAppearance {
     pub show_effect_fallback_message: bool,
 }
 
-/// All supported DLCs in CRAS.
-#[repr(C)]
-#[derive(Clone, Copy, PartialEq, Hash, Eq, Debug)]
-pub enum CrasDlcId {
-    CrasDlcSrBt,
-    CrasDlcNcAp,
-    CrasDlcIntelligoBeamforming,
-}
-
-// The list of DLCs that are installed automatically.
-pub const MANAGED_DLCS: &[CrasDlcId] = &[
-    CrasDlcId::CrasDlcSrBt,
-    CrasDlcId::CrasDlcNcAp,
-    CrasDlcId::CrasDlcIntelligoBeamforming,
-];
-
-pub const NUM_CRAS_DLCS: usize = 3;
-// Assert that NUM_CRAS_DLCS is updated.
-// We cannot assign MANAGED_DLCS.len() to NUM_CRAS_DLCS because cbindgen does
-// not seem to understand it.
-static_assertions::const_assert_eq!(NUM_CRAS_DLCS, MANAGED_DLCS.len());
-
-pub const CRAS_DLC_ID_STRING_MAX_LENGTH: i32 = 50;
-impl CrasDlcId {
-    pub fn as_str(&self) -> &'static str {
-        match self {
-            // The length of these strings should be bounded by
-            // CRAS_DLC_ID_STRING_MAX_LENGTH
-            CrasDlcId::CrasDlcSrBt => "sr-bt-dlc",
-            CrasDlcId::CrasDlcNcAp => "nc-ap-dlc",
-            CrasDlcId::CrasDlcIntelligoBeamforming => "intelligo-beamforming-dlc",
-        }
-    }
-}
-
-impl TryFrom<&str> for CrasDlcId {
-    type Error = anyhow::Error;
-
-    fn try_from(value: &str) -> anyhow::Result<Self> {
-        for dlc in MANAGED_DLCS {
-            if dlc.as_str() == value {
-                return Ok(dlc.clone());
-            }
-        }
-        bail!("unknown DLC {value}");
-    }
-}
-
-impl Display for CrasDlcId {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(self.as_str())
-    }
-}
+pub const SR_BT_DLC: &str = "sr-bt-dlc";
+pub const NC_AP_DLC: &str = "nc-ap-dlc";
 
 #[cfg(test)]
 mod tests {

cras/server/platform/dlc/Cargo.toml

@@ -16,6 +16,7 @@ protobuf = { workspace = true }
 static_assertions = { workspace = true }
 system_api = { workspace = true, optional = true }
 thiserror = { workspace = true }
+zerocopy = { workspace = true }
 
 [features]
 dlc = ["system_api"]

cras/server/platform/dlc/dlc.h

@@ -19,33 +19,38 @@ extern "C" {
 #include <stdlib.h>
 #include "cras/common/rust_common.h"
 
-struct CrasDlcDownloadConfig {
-  bool dlcs_to_download[NUM_CRAS_DLCS];
+/**
+ * This type exists as an alternative to heap-allocated C-strings.
+ *
+ * This type, as a simple value, is free of ownership or memory leak issues,
+ * when we pass this in a callback we don't have to worry about who should free the string.
+ */
+struct CrasDlcId128 {
+  char id[128];
 };
 
-typedef int (*DlcInstallOnSuccessCallback)(enum CrasDlcId, int32_t);
+typedef int (*DlcInstallOnSuccessCallback)(struct CrasDlcId128 id, int32_t elapsed_seconds);
 
-typedef int (*DlcInstallOnFailureCallback)(enum CrasDlcId, int32_t);
+typedef int (*DlcInstallOnFailureCallback)(struct CrasDlcId128 id, int32_t elapsed_seconds);
 
 /**
- * Returns `true` if the DLC package is ready for use, otherwise
- * returns `false`.
+ * Returns `true` if sr-bt-dlc is available.
  */
-bool cras_dlc_is_available(enum CrasDlcId id);
+bool cras_dlc_is_sr_bt_available(void);
 
 /**
- * Returns the root path of the DLC package.
+ * Returns the root path of sr-bt-dlc.
  * The returned string should be freed with cras_rust_free_string.
  */
-char *cras_dlc_get_root_path(enum CrasDlcId id);
+char *cras_dlc_get_sr_bt_root_path(void);
 
 /**
  * Overrides the DLC state for DLC `id`.
  *
  * # Safety
  * root_path must be a valid NULL terminated UTF-8 string.
  */
-void cras_dlc_override_state_for_testing(enum CrasDlcId id, bool installed, const char *root_path);
+void cras_dlc_override_sr_bt_for_testing(bool installed, const char *root_path);
 
 /**
  * Reset all DLC overrides.
@@ -55,8 +60,7 @@ void cras_dlc_reset_overrides_for_testing(void);
 /**
  * Start a thread to download all DLCs.
  */
-void download_dlcs_until_installed_with_thread(struct CrasDlcDownloadConfig download_config,
-                                               DlcInstallOnSuccessCallback dlc_install_on_success_callback,
+void download_dlcs_until_installed_with_thread(DlcInstallOnSuccessCallback dlc_install_on_success_callback,
                                                DlcInstallOnFailureCallback dlc_install_on_failure_callback);
 
 #endif  /* CRAS_SERVER_PLATFORM_DLC_DLC_H_ */

cras/server/platform/dlc/src/bindings.rs

@@ -8,31 +8,30 @@ use std::ffi::CString;
 use std::ptr;
 use std::thread;
 
+use cras_common::types_internal::SR_BT_DLC;
+
 use super::get_dlc_state_cached;
-use super::CrasDlcId;
 use super::Result;
-use crate::download_dlcs_init;
 use crate::download_dlcs_until_installed;
 use crate::DlcInstallOnFailureCallback;
 use crate::DlcInstallOnSuccessCallback;
 
-fn get_dlc_root_path(id: CrasDlcId) -> Result<CString> {
+fn get_dlc_root_path(id: &str) -> Result<CString> {
     let dlc_state = get_dlc_state_cached(id);
     CString::new(dlc_state.root_path).map_err(|e| e.into())
 }
 
-/// Returns `true` if the DLC package is ready for use, otherwise
-/// returns `false`.
+/// Returns `true` if sr-bt-dlc is available.
 #[no_mangle]
-pub extern "C" fn cras_dlc_is_available(id: CrasDlcId) -> bool {
-    get_dlc_state_cached(id).installed
+pub extern "C" fn cras_dlc_is_sr_bt_available() -> bool {
+    get_dlc_state_cached(SR_BT_DLC).installed
 }
 
-/// Returns the root path of the DLC package.
+/// Returns the root path of sr-bt-dlc.
 /// The returned string should be freed with cras_rust_free_string.
 #[no_mangle]
-pub extern "C" fn cras_dlc_get_root_path(id: CrasDlcId) -> *mut c_char {
-    match get_dlc_root_path(id) {
+pub extern "C" fn cras_dlc_get_sr_bt_root_path() -> *mut c_char {
+    match get_dlc_root_path(SR_BT_DLC) {
         Ok(root_path) => root_path.into_raw(),
         Err(_) => ptr::null_mut(),
     }
@@ -43,8 +42,7 @@ pub extern "C" fn cras_dlc_get_root_path(id: CrasDlcId) -> *mut c_char {
 /// # Safety
 /// root_path must be a valid NULL terminated UTF-8 string.
 #[no_mangle]
-pub unsafe extern "C" fn cras_dlc_override_state_for_testing(
-    id: CrasDlcId,
+pub unsafe extern "C" fn cras_dlc_override_sr_bt_for_testing(
     installed: bool,
     root_path: *const c_char,
 ) {
@@ -57,7 +55,7 @@ pub unsafe extern "C" fn cras_dlc_override_state_for_testing(
             .into()
     };
     crate::override_state_for_testing(
-        id,
+        SR_BT_DLC,
         crate::State {
             installed,
             root_path,
@@ -74,17 +72,13 @@ pub extern "C" fn cras_dlc_reset_overrides_for_testing() {
 /// Start a thread to download all DLCs.
 #[no_mangle]
 pub extern "C" fn download_dlcs_until_installed_with_thread(
-    download_config: super::CrasDlcDownloadConfig,
     dlc_install_on_success_callback: DlcInstallOnSuccessCallback,
     dlc_install_on_failure_callback: DlcInstallOnFailureCallback,
 ) {
-    download_dlcs_init(&download_config);
-
     thread::Builder::new()
         .name("cras-dlc".into())
         .spawn(move || {
             download_dlcs_until_installed(
-                download_config,
                 dlc_install_on_success_callback,
                 dlc_install_on_failure_callback,
             )

cras/server/platform/dlc/src/chromiumos.rs

@@ -12,7 +12,6 @@ use system_api::dlcservice::dlc_state::State;
 use system_api::dlcservice::DlcState;
 use system_api::dlcservice::InstallRequest;
 
-use super::CrasDlcId;
 use super::Result;
 
 const DBUS_TIMEOUT: Duration = Duration::from_millis(500);
@@ -34,7 +33,7 @@ impl super::ServiceTrait for Service {
         Ok(Self)
     }
 
-    fn install(&mut self, id: CrasDlcId) -> Result<()> {
+    fn install(&mut self, id: &str) -> Result<()> {
         let connection = Connection::new_system()?;
         let conn_path = get_dlcservice_connection_path(&connection);
 
@@ -44,11 +43,11 @@ impl super::ServiceTrait for Service {
         Ok(conn_path.install(request.write_to_bytes()?)?)
     }
 
-    fn get_dlc_state(&mut self, id: CrasDlcId) -> Result<crate::State> {
+    fn get_dlc_state(&mut self, id: &str) -> Result<crate::State> {
         let connection = Connection::new_system()?;
         let conn_path = get_dlcservice_connection_path(&connection);
 
-        let res = conn_path.get_dlc_state(id.as_str())?;
+        let res = conn_path.get_dlc_state(id)?;
         let mut dlc_state = DlcState::new();
         dlc_state.merge_from_bytes(&res)?;
         Ok(crate::State {