Remove re-initialization of HAL on Enable
Bug: 216357072
Test: UwbManagerTest
Change-Id: I8efbec076d14d175bd28c66227714ac667c63776
diff --git a/src/rust/adaptation/mod.rs b/src/rust/adaptation/mod.rs
index c144c39..3742b0d 100644
--- a/src/rust/adaptation/mod.rs
+++ b/src/rust/adaptation/mod.rs
@@ -89,58 +89,36 @@
}
pub trait UwbAdaptation {
- fn initialize(&mut self);
fn finalize(&mut self, exit_status: bool);
fn hal_open(&self);
fn hal_close(&self);
fn core_initialization(&self) -> Result<(), UwbErr>;
fn session_initialization(&self, session_id: i32) -> Result<(), UwbErr>;
- fn send_uci_message(&self, data: &[u8]);
+ fn send_uci_message(&self, data: &[u8]) -> Result<(), UwbErr>;
}
#[derive(Clone)]
pub struct UwbAdaptationImpl {
- hal: Option<Strong<dyn IUwbChip>>,
+ hal: Strong<dyn IUwbChip>,
rsp_sender: mpsc::UnboundedSender<HalCallback>,
}
impl UwbAdaptationImpl {
- pub fn new(
- hal: Option<Strong<dyn IUwbChip>>,
- rsp_sender: mpsc::UnboundedSender<HalCallback>,
- ) -> Self {
- Self { hal, rsp_sender }
- }
-
- fn initialize_hal_device_context(&mut self) {
- // TODO: If we can initialize this in new(), we can properly error if it fails and remove
- // the checks in the functions below.
- self.hal = get_hal_service();
- if (self.hal.is_none()) {
- info!("Failed to retrieve the UWB HAL!");
- }
+ pub fn new(rsp_sender: mpsc::UnboundedSender<HalCallback>) -> Result<Self, UwbErr> {
+ let hal = get_hal_service().ok_or(UwbErr::HalUnavailable)?;
+ Ok(UwbAdaptationImpl { hal, rsp_sender })
}
fn get_supported_android_uci_version(&self) -> Result<i32, UwbErr> {
- if let Some(hal) = &self.hal {
- return Ok(hal.getSupportedAndroidUciVersion()?);
- }
- Err(UwbErr::failed())
+ Ok(self.hal.getSupportedAndroidUciVersion()?)
}
fn get_supported_android_capabilities(&self) -> Result<i64, UwbErr> {
- if let Some(hal) = &self.hal {
- return Ok(hal.getSupportedAndroidCapabilities()?);
- }
- Err(UwbErr::failed())
+ Ok(self.hal.getSupportedAndroidCapabilities()?)
}
}
impl UwbAdaptation for UwbAdaptationImpl {
- fn initialize(&mut self) {
- self.initialize_hal_device_context();
- }
-
fn finalize(&mut self, exit_status: bool) {}
fn hal_open(&self) {
@@ -148,39 +126,25 @@
UwbClientCallback { rsp_sender: self.rsp_sender.clone() },
BinderFeatures::default(),
);
- if let Some(hal) = &self.hal {
- hal.open(&m_cback);
- } else {
- warn!("Failed to open HAL");
- }
+ self.hal.open(&m_cback);
}
fn hal_close(&self) {
- if let Some(hal) = &self.hal {
- hal.close();
- }
+ self.hal.close();
}
fn core_initialization(&self) -> Result<(), UwbErr> {
- if let Some(hal) = &self.hal {
- return Ok(hal.coreInit()?);
- }
- Err(UwbErr::failed())
+ Ok(self.hal.coreInit()?)
}
fn session_initialization(&self, session_id: i32) -> Result<(), UwbErr> {
- if let Some(hal) = &self.hal {
- return Ok(hal.sessionInit(session_id)?);
- }
- Err(UwbErr::failed())
+ Ok(self.hal.sessionInit(session_id)?)
}
- fn send_uci_message(&self, data: &[u8]) {
- if let Some(hal) = &self.hal {
- hal.sendUciMessage(data);
- } else {
- warn!("Failed to send uci message");
- }
+ fn send_uci_message(&self, data: &[u8]) -> Result<(), UwbErr> {
+ self.hal.sendUciMessage(data)?;
+ // TODO should we be validating the returned number?
+ Ok(())
}
}
@@ -198,7 +162,6 @@
#[cfg(test)]
impl UwbAdaptation for MockUwbAdaptation {
- fn initialize(&mut self) {}
fn finalize(&mut self, exit_status: bool) {}
fn hal_open(&self) {}
fn hal_close(&self) {}
@@ -212,7 +175,9 @@
fn session_initialization(&self, session_id: i32) -> Result<(), UwbErr> {
Ok(())
}
- fn send_uci_message(&self, data: &[u8]) {}
+ fn send_uci_message(&self, data: &[u8]) -> Result<(), UwbErr> {
+ Ok(())
+ }
}
#[cfg(test)]
diff --git a/src/rust/error.rs b/src/rust/error.rs
index 055d943..e0a6293 100644
--- a/src/rust/error.rs
+++ b/src/rust/error.rs
@@ -51,6 +51,8 @@
InvalidArgs,
#[error("Exit")]
Exit,
+ #[error("Could not connect to HAL")]
+ HalUnavailable,
#[error("Unknown error")]
Undefined,
}
diff --git a/src/rust/uci/mod.rs b/src/rust/uci/mod.rs
index 81a7548..72e9845 100644
--- a/src/rust/uci/mod.rs
+++ b/src/rust/uci/mod.rs
@@ -308,29 +308,11 @@
log::debug!("Received non blocking cmd {:?}", cmd);
match cmd {
JNICommand::Enable => {
- // TODO: This mimics existing behavior, but I think we've got a few
- // issues here:
- // * We've got two different initialization sites (Enable *and*
- // adaptation construction)
- // * We have multiple functions required to finish building a
- // correct Enable (so there are bad states to leave it in)
- // * We have Disable, but the adaptation isn't optional, so we
- // will end up with an invalid but still present adaptation.
- //
- // A future patch should probably make a single constructor for
- // everything, and it should probably be called here rather than
- // mutating an existing adaptation. The adaptation should be made
- // optional.
- if let Some(adaptation) = Arc::get_mut(&mut self.adaptation) {
- adaptation.initialize();
- adaptation.hal_open();
- adaptation
- .core_initialization()
- .unwrap_or_else(|e| error!("Error invoking core init HAL API : {:?}", e));
- self.set_state(UwbState::W4HalOpen);
- } else {
- error!("Attempted to enable Uci while it was still in use.");
- }
+ self.adaptation.hal_open();
+ self.adaptation
+ .core_initialization()
+ .unwrap_or_else(|e| error!("Error invoking core init HAL API : {:?}", e));
+ self.set_state(UwbState::W4HalOpen);
}
JNICommand::Disable(graceful) => {
self.set_state(UwbState::W4HalClose);
@@ -384,7 +366,6 @@
// Handles a single message from JNI or the HAL.
async fn drive_once(&mut self) -> Result<()> {
- // TODO: Handle messages for real instead of just logging them.
select! {
Some(()) = option_future(self.response_channel.as_ref()
.map(|(_, retryer)| retryer.command_failed())) => {
@@ -397,8 +378,6 @@
Some((cmd, tx)) = self.cmd_receiver.recv(), if self.can_process_cmd() => {
match tx {
Some(tx) => { // Blocking JNI commands processing.
- // TODO: If we do something similar to communication to the HAL (using a channel
- // to hide the asynchrony, we can remove the field and make this straight line code.
self.handle_blocking_jni_cmd(tx, cmd)?;
},
None => { // Non Blocking JNI commands processing.
@@ -472,7 +451,7 @@
impl Dispatcher {
pub fn new<T: 'static + EventManager + std::marker::Send>(event_manager: T) -> Result<Self> {
let (rsp_sender, rsp_receiver) = mpsc::unbounded_channel::<HalCallback>();
- let adaptation: SyncUwbAdaptation = Box::new(UwbAdaptationImpl::new(None, rsp_sender));
+ let adaptation: SyncUwbAdaptation = Box::new(UwbAdaptationImpl::new(rsp_sender)?);
Self::new_with_args(event_manager, adaptation, rsp_receiver)
}
@@ -494,6 +473,7 @@
info!("initializing dispatcher");
let (cmd_sender, cmd_receiver) =
mpsc::unbounded_channel::<(JNICommand, Option<UciResponseHandle>)>();
+
// We create a new thread here both to avoid reusing the Java service thread and because
// binder threads will call into this.
let runtime = Builder::new_multi_thread()
