From bc1848d25c0d530ee8e08655b624ba92eac6a3e6 Mon Sep 17 00:00:00 2001 From: cranelw Date: Thu, 15 Aug 2024 09:32:35 +0000 Subject: [PATCH] Move CrasDlcId into types_internal.rs This is mainly for s2 to access CrasDlcId and avoid cyclic dependency. Before this change, dlc depends on s2, and CrasDlcId is defined in dlc. After this change, dlc depends on types_internal and s2, and s2 can depend on types_internal. Per discussion, we decided not to make it a crate to keep the original design: 1 crate with 1 header. BUG=b:359803173 TEST=bazel test //... Change-Id: I474fc294d5e6e0d6db4f7378c6b7ebd7d2ea41e5 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/5790042 Tested-by: Cranel W Commit-Queue: Cranel W Tested-by: chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com Reviewed-by: Li-Yu Yu --- Cargo.Bazel.lock | 6 ++- Cargo.lock | 2 + cras/common/BUILD.bazel | 5 +- cras/common/Cargo.toml | 1 + cras/common/rust_common.h | 13 +++++ cras/common/src/types_internal.rs | 56 ++++++++++++++++++++ cras/server/platform/dlc/BUILD.bazel | 15 +++++- cras/server/platform/dlc/Cargo.toml | 1 + cras/server/platform/dlc/dlc.h | 14 +---- cras/server/platform/dlc/src/lib.rs | 61 ++-------------------- cras/src/server/cras_server_metrics.c | 1 + cras/src/server/rust/src/cras_processor.rs | 2 +- rules/cbindgen/src/main.rs | 14 ++++- 13 files changed, 114 insertions(+), 77 deletions(-) diff --git a/Cargo.Bazel.lock b/Cargo.Bazel.lock index 5c80df115..41e2a2bc6 100644 --- a/Cargo.Bazel.lock +++ b/Cargo.Bazel.lock @@ -1,5 +1,5 @@ { - "checksum": "fade509b4eee7c50395810a98977ace70cfd3e4a024a4716fef35e79a100a004", + "checksum": "ec8622ad7e424559f85247a40b5f3b070428ba7a3012b6ed374ae6a1f5e86867", "crates": { "addr2line 0.20.0": { "name": "addr2line", @@ -2196,6 +2196,10 @@ "id": "serde_json 1.0.96", "target": "serde_json" }, + { + "id": "static_assertions 1.1.0", + "target": "static_assertions" + }, { "id": "syslog 6.0.1", "target": "syslog" diff --git a/Cargo.lock b/Cargo.lock index 501cd4ad2..401dd400c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -365,6 +365,7 @@ dependencies = [ "once_cell", "openssl", "serde_json", + "static_assertions", "syslog", ] @@ -373,6 +374,7 @@ name = "cras_dlc" version = "0.1.0" dependencies = [ "anyhow", + "cras_common", "cras_s2", "dbus", "libc", diff --git a/cras/common/BUILD.bazel b/cras/common/BUILD.bazel index 034fbe608..6a0712c1e 100644 --- a/cras/common/BUILD.bazel +++ b/cras/common/BUILD.bazel @@ -33,7 +33,10 @@ cras_rust_library( ), crate_name = "cras_common", edition = "2021", - visibility = ["//cras/src/server/rust:__pkg__"], + visibility = [ + "//cras/server/platform:__subpackages__", + "//cras/src/server/rust:__pkg__", + ], deps = [ "@pkg_config//openssl", ] + all_crate_deps(normal = True), diff --git a/cras/common/Cargo.toml b/cras/common/Cargo.toml index 52287951d..c2245e816 100644 --- a/cras/common/Cargo.toml +++ b/cras/common/Cargo.toml @@ -14,4 +14,5 @@ nix = { version = "0.28.0", features = ["process"] } once_cell = "1.17.0" openssl = "0.10.48" serde_json = "1.0.64" +static_assertions = "1.1.0" syslog = "6.0.1" diff --git a/cras/common/rust_common.h b/cras/common/rust_common.h index 486da7832..2f76f798b 100644 --- a/cras/common/rust_common.h +++ b/cras/common/rust_common.h @@ -18,6 +18,19 @@ extern "C" { #include #include +#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, diff --git a/cras/common/src/types_internal.rs b/cras/common/src/types_internal.rs index 2b0cfc26d..46acd8f2e 100644 --- a/cras/common/src/types_internal.rs +++ b/cras/common/src/types_internal.rs @@ -4,7 +4,9 @@ use std::borrow::Cow; use std::ffi::CString; +use std::fmt::Display; +use anyhow::bail; use bitflags::bitflags; use itertools::Itertools; @@ -43,3 +45,57 @@ pub extern "C" fn cras_stream_active_ap_effects_string( .unwrap() .into_raw() } + +/// 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 { + 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()) + } +} diff --git a/cras/server/platform/dlc/BUILD.bazel b/cras/server/platform/dlc/BUILD.bazel index 30d74af65..d7958a9e3 100644 --- a/cras/server/platform/dlc/BUILD.bazel +++ b/cras/server/platform/dlc/BUILD.bazel @@ -11,8 +11,12 @@ cras_rust_library( name = "dlc", srcs = glob(include = ["src/*.rs"]), crate_name = "cras_dlc", - visibility = ["//cras/src/server/rust:__pkg__"], + visibility = [ + "//cras/server/platform:__pkg__", + "//cras/src/server/rust:__pkg__", + ], deps = all_crate_deps() + [ + "//cras/common:rust_common", "//cras/server/s2", "@pkg_config//dbus-1", ], @@ -24,11 +28,18 @@ cras_cbindgen( srcs = glob(include = ["src/*.rs"]), out = "dlc.h", copyright_year = 2023, + extra_args = [ + "--with-include=cras/common/rust_common.h", + "--add-keyword-enum", + ], ) cc_library( name = "cc", hdrs = ["dlc.h"], visibility = ["//visibility:public"], - deps = [":dlc"], + deps = [ + ":dlc", + "//cras/common:rust_common_cc", + ], ) diff --git a/cras/server/platform/dlc/Cargo.toml b/cras/server/platform/dlc/Cargo.toml index 1f22cbd2a..b36d9317b 100644 --- a/cras/server/platform/dlc/Cargo.toml +++ b/cras/server/platform/dlc/Cargo.toml @@ -6,6 +6,7 @@ edition = "2021" [dependencies] anyhow = "1.0.32" +cras_common = "*" cras_s2 = "*" dbus = "0.9" libc = "0.2.44" diff --git a/cras/server/platform/dlc/dlc.h b/cras/server/platform/dlc/dlc.h index 31fb23f0a..5c0283018 100644 --- a/cras/server/platform/dlc/dlc.h +++ b/cras/server/platform/dlc/dlc.h @@ -17,19 +17,7 @@ extern "C" { #include #include #include - -#define NUM_CRAS_DLCS 3 - -#define CRAS_DLC_ID_STRING_MAX_LENGTH 50 - -/** - * All supported DLCs in CRAS. - */ -enum CrasDlcId { - CrasDlcSrBt, - CrasDlcNcAp, - CrasDlcIntelligoBeamforming, -}; +#include "cras/common/rust_common.h" struct CrasDlcDownloadConfig { bool dlcs_to_download[NUM_CRAS_DLCS]; diff --git a/cras/server/platform/dlc/src/lib.rs b/cras/server/platform/dlc/src/lib.rs index 283d4a588..1e2954f5d 100644 --- a/cras/server/platform/dlc/src/lib.rs +++ b/cras/server/platform/dlc/src/lib.rs @@ -8,12 +8,11 @@ mod chromiumos; mod stub; use std::collections::HashMap; -use std::fmt::Display; use std::sync::Mutex; use std::thread::sleep; use std::time; -use anyhow::bail; +use cras_common::types_internal::CrasDlcId; use once_cell::sync::Lazy; use thiserror::Error; @@ -41,60 +40,6 @@ pub enum Error { pub type Result = std::result::Result; -/// 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. -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 { - 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 { - 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()) - } -} - trait ServiceTrait: Sized { fn new() -> Result; fn install(&mut self, id: CrasDlcId) -> Result<()>; @@ -152,7 +97,7 @@ type CrasServerMetricsDlcInstallRetriedTimesOnSuccessFunc = #[repr(C)] pub struct CrasDlcDownloadConfig { - pub dlcs_to_download: [bool; NUM_CRAS_DLCS], + pub dlcs_to_download: [bool; cras_common::types_internal::NUM_CRAS_DLCS], } fn download_dlcs_until_installed( @@ -164,7 +109,7 @@ fn download_dlcs_until_installed( let mut todo: Vec<_> = download_config .dlcs_to_download .iter() - .zip(MANAGED_DLCS.iter()) + .zip(cras_common::types_internal::MANAGED_DLCS.iter()) .filter_map(|(download, id)| if *download { Some(id) } else { None }) .collect(); for retry_count in 0..i32::MAX { diff --git a/cras/src/server/cras_server_metrics.c b/cras/src/server/cras_server_metrics.c index da33eca99..5cf7b93a8 100644 --- a/cras/src/server/cras_server_metrics.c +++ b/cras/src/server/cras_server_metrics.c @@ -13,6 +13,7 @@ #include #include +#include "cras/common/rust_common.h" #include "cras/server/main_message.h" #include "cras/server/platform/dlc/dlc.h" #include "cras/src/common/cras_metrics.h" diff --git a/cras/src/server/rust/src/cras_processor.rs b/cras/src/server/rust/src/cras_processor.rs index 21be7e7a1..5667d2fc0 100644 --- a/cras/src/server/rust/src/cras_processor.rs +++ b/cras/src/server/rust/src/cras_processor.rs @@ -24,8 +24,8 @@ use audio_processor::processors::ThreadedProcessor; use audio_processor::AudioProcessor; use audio_processor::Format; use audio_processor::ProcessorVec; +use cras_common::types_internal::CrasDlcId; use cras_dlc::get_dlc_state_cached; -use cras_dlc::CrasDlcId; mod processor_override; diff --git a/rules/cbindgen/src/main.rs b/rules/cbindgen/src/main.rs index 87a81952f..aea064d31 100644 --- a/rules/cbindgen/src/main.rs +++ b/rules/cbindgen/src/main.rs @@ -53,6 +53,12 @@ struct Command { #[arg(long, default_value = "2023")] copyright_year: u32, + + #[arg( + long, + help = "Add the `enum` keyword to generated enums. Useful if the enums are not automatically prefixed with `enum`." + )] + add_keyword_enum: bool, } fn main() { @@ -94,9 +100,15 @@ fn main() { .rename_item("DRC", "drc") .rename_item("DRCComponent", "drc_component") .rename_item("DRC_PARAM", "drc_param") - .include_item("DRC_PARAM"); + .include_item("DRC_PARAM") + .include_item("CrasDlcId"); let mut b = b; + + if c.add_keyword_enum { + b = b.rename_item("CrasDlcId", "enum CrasDlcId"); + } + for src in c.with_src { b = b.with_src(src); }