From ea05baf3386ef96cba90e7811bcbb015273be046 Mon Sep 17 00:00:00 2001 From: Jesus Sanchez-Palencia Date: Mon, 26 Jun 2023 17:23:53 -0700 Subject: [PATCH 1/2] lights aidl: Add required @Rust derive statements The new LightsService example is written in Rust and is being expanded to include a state as part of the service. This required that HwLight and HwLightState derived from the Copy and Clone traits, so here we are updating the AIDL Rust bindings. This is not an API change, so in order to avoid having to bump the AIDL API version for this HAL we used the hash_gen.sh script as below: $ m android.hardware.light-update-api $ m android.hardware.light-freeze-api $ vim light/aidl/Android.bp # removed frozen_api argument and the newly created version 3 block $ cp -r aidl_api/android.hardware.light/3/* aidl_api/android.hardware.light/2/ $ rm -rf aidl_api/android.hardware.light/3/ $ ./system/tools/aidl/build/hash_gen.sh \ hardware/interfaces/light/aidl/aidl_api/android.hardware.light/2/ \ 1 \ hardware/interfaces/light/aidl/aidl_api/android.hardware.light/2/.hash Tested: Verified that the build is passing and used with the next CL. Bug: 286106270 Change-Id: I1400ec1db1e75595176a5656d6688df9457153d4 --- light/aidl/aidl_api/android.hardware.light/2/.hash | 1 + .../2/android/hardware/light/HwLight.aidl | 2 +- .../2/android/hardware/light/HwLightState.aidl | 2 +- .../current/android/hardware/light/HwLight.aidl | 2 +- .../current/android/hardware/light/HwLightState.aidl | 2 +- light/aidl/android/hardware/light/HwLight.aidl | 2 +- light/aidl/android/hardware/light/HwLightState.aidl | 2 +- 7 files changed, 7 insertions(+), 6 deletions(-) diff --git a/light/aidl/aidl_api/android.hardware.light/2/.hash b/light/aidl/aidl_api/android.hardware.light/2/.hash index d27f4ad9d4..2d4e7f0a65 100644 --- a/light/aidl/aidl_api/android.hardware.light/2/.hash +++ b/light/aidl/aidl_api/android.hardware.light/2/.hash @@ -1 +1,2 @@ c8b1e8ebb88c57dcb2c350a8d9b722e77dd864c8 +c7d3d941d303c70d1c22759a0b09e41930c1cddb diff --git a/light/aidl/aidl_api/android.hardware.light/2/android/hardware/light/HwLight.aidl b/light/aidl/aidl_api/android.hardware.light/2/android/hardware/light/HwLight.aidl index 25a2dce37d..5ac2a34305 100644 --- a/light/aidl/aidl_api/android.hardware.light/2/android/hardware/light/HwLight.aidl +++ b/light/aidl/aidl_api/android.hardware.light/2/android/hardware/light/HwLight.aidl @@ -32,7 +32,7 @@ // later when a module using the interface is updated, e.g., Mainline modules. package android.hardware.light; -@VintfStability +@RustDerive(Clone=true, Copy=true) @VintfStability parcelable HwLight { int id; int ordinal; diff --git a/light/aidl/aidl_api/android.hardware.light/2/android/hardware/light/HwLightState.aidl b/light/aidl/aidl_api/android.hardware.light/2/android/hardware/light/HwLightState.aidl index 40e520b796..2878ce256f 100644 --- a/light/aidl/aidl_api/android.hardware.light/2/android/hardware/light/HwLightState.aidl +++ b/light/aidl/aidl_api/android.hardware.light/2/android/hardware/light/HwLightState.aidl @@ -32,7 +32,7 @@ // later when a module using the interface is updated, e.g., Mainline modules. package android.hardware.light; -@VintfStability +@RustDerive(Clone=true, Copy=true) @VintfStability parcelable HwLightState { int color; android.hardware.light.FlashMode flashMode; diff --git a/light/aidl/aidl_api/android.hardware.light/current/android/hardware/light/HwLight.aidl b/light/aidl/aidl_api/android.hardware.light/current/android/hardware/light/HwLight.aidl index 25a2dce37d..5ac2a34305 100644 --- a/light/aidl/aidl_api/android.hardware.light/current/android/hardware/light/HwLight.aidl +++ b/light/aidl/aidl_api/android.hardware.light/current/android/hardware/light/HwLight.aidl @@ -32,7 +32,7 @@ // later when a module using the interface is updated, e.g., Mainline modules. package android.hardware.light; -@VintfStability +@RustDerive(Clone=true, Copy=true) @VintfStability parcelable HwLight { int id; int ordinal; diff --git a/light/aidl/aidl_api/android.hardware.light/current/android/hardware/light/HwLightState.aidl b/light/aidl/aidl_api/android.hardware.light/current/android/hardware/light/HwLightState.aidl index 40e520b796..2878ce256f 100644 --- a/light/aidl/aidl_api/android.hardware.light/current/android/hardware/light/HwLightState.aidl +++ b/light/aidl/aidl_api/android.hardware.light/current/android/hardware/light/HwLightState.aidl @@ -32,7 +32,7 @@ // later when a module using the interface is updated, e.g., Mainline modules. package android.hardware.light; -@VintfStability +@RustDerive(Clone=true, Copy=true) @VintfStability parcelable HwLightState { int color; android.hardware.light.FlashMode flashMode; diff --git a/light/aidl/android/hardware/light/HwLight.aidl b/light/aidl/android/hardware/light/HwLight.aidl index 43fdb4bf81..8db32cc043 100644 --- a/light/aidl/android/hardware/light/HwLight.aidl +++ b/light/aidl/android/hardware/light/HwLight.aidl @@ -22,7 +22,7 @@ import android.hardware.light.LightType; * A description of a single light. Multiple lights can map to the same physical * LED. Separate physical LEDs are always represented by separate instances. */ -@VintfStability +@RustDerive(Clone=true, Copy=true) @VintfStability parcelable HwLight { /** * Integer ID used for controlling this light diff --git a/light/aidl/android/hardware/light/HwLightState.aidl b/light/aidl/android/hardware/light/HwLightState.aidl index 24d3250887..3ba6c7874c 100644 --- a/light/aidl/android/hardware/light/HwLightState.aidl +++ b/light/aidl/android/hardware/light/HwLightState.aidl @@ -25,7 +25,7 @@ import android.hardware.light.FlashMode; * Not all lights must support all parameters. If you * can do something backward-compatible, do it. */ -@VintfStability +@RustDerive(Clone=true, Copy=true) @VintfStability parcelable HwLightState { /** * The color of the LED in ARGB. From c0544589b81a846850d02d68db23b3e8338d0b03 Mon Sep 17 00:00:00 2001 From: Jesus Sanchez-Palencia Date: Mon, 26 Jun 2023 17:28:06 -0700 Subject: [PATCH 2/2] lights: Add state to the example service Add a Light struct to the example service so it can hold a HwLight and its associated HwLightState. Also added a HashMap of ids -> Light to the LightService and updated the HAL methods implementation to make the service stateful. Now instantiating a LightService requires that a list of HwLights is provided, so a default implementation was provided as a convenience. The android.hardware.light rust API version had to be bumped since we rely on new derive statements added to the AIDL files. Tested: built VtsHalLightTargetTest and ran in Cuttlefish Bug: 286106270 Change-Id: Id2b17a6a2290295c7b0b5405ac9815eaa28303c6 --- light/aidl/default/lights.rs | 55 ++++++++++++++++++++++++++---------- light/aidl/default/main.rs | 2 +- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/light/aidl/default/lights.rs b/light/aidl/default/lights.rs index 60e98d72bc..6c8aa3fc27 100644 --- a/light/aidl/default/lights.rs +++ b/light/aidl/default/lights.rs @@ -15,6 +15,9 @@ */ //! This module implements the ILights AIDL interface. +use std::collections::HashMap; +use std::sync::Mutex; + use log::info; use android_hardware_light::aidl::android::hardware::light::{ @@ -23,33 +26,55 @@ use android_hardware_light::aidl::android::hardware::light::{ use binder::{ExceptionCode, Interface, Status}; +struct Light { + hw_light: HwLight, + state: HwLightState, +} + +const NUM_DEFAULT_LIGHTS: i32 = 3; + /// Defined so we can implement the ILights AIDL interface. -pub struct LightsService; +pub struct LightsService { + lights: Mutex>, +} impl Interface for LightsService {} -const NUM_DEFAULT_LIGHTS: i32 = 3; +impl LightsService { + fn new(hw_lights: impl IntoIterator) -> Self { + let mut lights_map = HashMap::new(); + + for hw_light in hw_lights { + lights_map.insert(hw_light.id, Light { hw_light, state: Default::default() }); + } + + Self { lights: Mutex::new(lights_map) } + } +} + +impl Default for LightsService { + fn default() -> Self { + let id_mapping_closure = + |light_id| HwLight { id: light_id, ordinal: light_id, r#type: LightType::BACKLIGHT }; + + Self::new((1..=NUM_DEFAULT_LIGHTS).map(id_mapping_closure)) + } +} impl ILights for LightsService { fn setLightState(&self, id: i32, state: &HwLightState) -> binder::Result<()> { info!("Lights setting state for id={} to color {:x}", id, state.color); - if id <= 0 || id > NUM_DEFAULT_LIGHTS { - return Err(Status::new_exception(ExceptionCode::UNSUPPORTED_OPERATION, None)); - } - Ok(()) + if let Some(light) = self.lights.lock().unwrap().get_mut(&id) { + light.state = *state; + Ok(()) + } else { + Err(Status::new_exception(ExceptionCode::UNSUPPORTED_OPERATION, None)) + } } fn getLights(&self) -> binder::Result> { - let mut lights: Vec = Vec::with_capacity(NUM_DEFAULT_LIGHTS.try_into().unwrap()); - - for i in 1..=NUM_DEFAULT_LIGHTS { - let light = HwLight { id: i, ordinal: i, r#type: LightType::BACKLIGHT }; - - lights.push(light); - } - info!("Lights reporting supported lights"); - Ok(lights) + Ok(self.lights.lock().unwrap().values().map(|light| light.hw_light).collect()) } } diff --git a/light/aidl/default/main.rs b/light/aidl/default/main.rs index ffe33fe838..8f3247089b 100644 --- a/light/aidl/default/main.rs +++ b/light/aidl/default/main.rs @@ -35,7 +35,7 @@ fn main() { binder::ProcessState::set_thread_pool_max_thread_count(0); - let lights_service = LightsService; + let lights_service = LightsService::default(); let lights_service_binder = BnLights::new_binder(lights_service, BinderFeatures::default()); let service_name = format!("{}/default", LightsService::get_descriptor());