From 6f5459d8eb5fb8291e6ffc4ebf94c0e9eddf2996 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Mon, 16 Oct 2017 18:29:46 -0700 Subject: [PATCH] Update Light extension example to match docs. This light extension example was more of a POC. Now it matches the suggestions for minor version requirements and vendor extensions that are spelled out in publically available documentation. This includes things like: - taking advantage of namespacing rather than renaming things - adding documentation when extension differs from what a minor version extension should be - importing things specifically rather than dumping namespaces into the types.hal file Test: compilation only -- it doesn't do anything Change-Id: Ia1bf9baaddcb630cc1b6cd4fe272def0c7c492e9 Fixes: 67787567 --- tests/extension/light/2.0/Android.bp | 24 +++++++++---------- .../light/2.0/{IExtLight.hal => ILight.hal} | 12 ++++++---- tests/extension/light/2.0/default/Light.cpp | 19 ++++++--------- tests/extension/light/2.0/default/Light.h | 19 +++++++-------- tests/extension/light/2.0/types.hal | 16 ++++++++----- 5 files changed, 46 insertions(+), 44 deletions(-) rename tests/extension/light/2.0/{IExtLight.hal => ILight.hal} (74%) diff --git a/tests/extension/light/2.0/Android.bp b/tests/extension/light/2.0/Android.bp index 796e2565f6..612c2d6db9 100644 --- a/tests/extension/light/2.0/Android.bp +++ b/tests/extension/light/2.0/Android.bp @@ -4,7 +4,7 @@ filegroup { name: "android.hardware.tests.extension.light@2.0_hal", srcs: [ "types.hal", - "IExtLight.hal", + "ILight.hal", ], } @@ -17,7 +17,7 @@ genrule { ], out: [ "android/hardware/tests/extension/light/2.0/types.cpp", - "android/hardware/tests/extension/light/2.0/ExtLightAll.cpp", + "android/hardware/tests/extension/light/2.0/LightAll.cpp", ], } @@ -31,11 +31,11 @@ genrule { out: [ "android/hardware/tests/extension/light/2.0/types.h", "android/hardware/tests/extension/light/2.0/hwtypes.h", - "android/hardware/tests/extension/light/2.0/IExtLight.h", - "android/hardware/tests/extension/light/2.0/IHwExtLight.h", - "android/hardware/tests/extension/light/2.0/BnHwExtLight.h", - "android/hardware/tests/extension/light/2.0/BpHwExtLight.h", - "android/hardware/tests/extension/light/2.0/BsExtLight.h", + "android/hardware/tests/extension/light/2.0/ILight.h", + "android/hardware/tests/extension/light/2.0/IHwLight.h", + "android/hardware/tests/extension/light/2.0/BnHwLight.h", + "android/hardware/tests/extension/light/2.0/BpHwLight.h", + "android/hardware/tests/extension/light/2.0/BsLight.h", ], } @@ -72,10 +72,10 @@ genrule { ":android.hardware.tests.extension.light@2.0_hal", ], out: [ + "android/hardware/tests/extension/light/V2_0/Brightness.java", "android/hardware/tests/extension/light/V2_0/Default.java", - "android/hardware/tests/extension/light/V2_0/ExtBrightness.java", - "android/hardware/tests/extension/light/V2_0/ExtLightState.java", - "android/hardware/tests/extension/light/V2_0/IExtLight.java", + "android/hardware/tests/extension/light/V2_0/LightState.java", + "android/hardware/tests/extension/light/V2_0/ILight.java", ], } @@ -102,7 +102,7 @@ genrule { ":android.hardware.tests.extension.light@2.0_hal", ], out: [ - "android/hardware/tests/extension/light/2.0/AExtLight.cpp", + "android/hardware/tests/extension/light/2.0/ALight.cpp", ], } @@ -114,7 +114,7 @@ genrule { ":android.hardware.tests.extension.light@2.0_hal", ], out: [ - "android/hardware/tests/extension/light/2.0/AExtLight.h", + "android/hardware/tests/extension/light/2.0/ALight.h", ], } diff --git a/tests/extension/light/2.0/IExtLight.hal b/tests/extension/light/2.0/ILight.hal similarity index 74% rename from tests/extension/light/2.0/IExtLight.hal rename to tests/extension/light/2.0/ILight.hal index 1515b869c0..94781c0838 100644 --- a/tests/extension/light/2.0/IExtLight.hal +++ b/tests/extension/light/2.0/ILight.hal @@ -19,17 +19,21 @@ // vendor partition. package android.hardware.tests.extension.light@2.0; -import android.hardware.light@2.0; +import android.hardware.light@2.0::ILight; +import android.hardware.light@2.0::Status; +import android.hardware.light@2.0::Type; -interface IExtLight extends android.hardware.light@2.0::ILight { +interface ILight extends android.hardware.light@2.0::ILight { /** * Set the provided lights to the provided values. * + * If this was a minor version extension, the recommended + * name would be setLight_2_1. + * * @param type logical light to set * @param state describes what the light should look like. * @return status result of applying state transformation. */ - setExtLight(Type type, ExtLightState state) generates (Status status); - + setLightExt(Type type, LightState state) generates (Status status); }; diff --git a/tests/extension/light/2.0/default/Light.cpp b/tests/extension/light/2.0/default/Light.cpp index d941e73e00..9df79248a7 100644 --- a/tests/extension/light/2.0/default/Light.cpp +++ b/tests/extension/light/2.0/default/Light.cpp @@ -24,18 +24,15 @@ namespace V2_0 { namespace implementation { // Methods from ::android::hardware::light::V2_0::ILight follow. -Return Light::setLight(Type type, const LightState& state) { +Return Light::setLight(Type type, const OldLightState& state) { // Forward types for new methods. - ExtLightState extState { - .state = state, - .interpolationOmega = - static_cast(Default::INTERPOLATION_OMEGA), - .brightness = // ExtBrightness inherits from Brightness - static_cast(state.brightnessMode) - }; + LightState extState{.state = state, + .interpolationOmega = static_cast(Default::INTERPOLATION_OMEGA), + .brightness = // Brightness inherits from Brightness + static_cast(state.brightnessMode)}; - return setExtLight(type, extState); + return setLightExt(type, extState); } Return Light::getSupportedTypes(getSupportedTypes_cb _hidl_cb) { @@ -52,9 +49,7 @@ Return Light::getSupportedTypes(getSupportedTypes_cb _hidl_cb) { } // Methods from ::android::hardware::example::extension::light::V2_0::ILight follow. -Return Light::setExtLight(Type /* type */, - const ExtLightState& /* state */) { - +Return Light::setLightExt(Type /* type */, const LightState& /* state */) { // ****************************************************** // Note: awesome proprietary hardware implementation here // ****************************************************** diff --git a/tests/extension/light/2.0/default/Light.h b/tests/extension/light/2.0/default/Light.h index dc2c5dd537..d178e75cbc 100644 --- a/tests/extension/light/2.0/default/Light.h +++ b/tests/extension/light/2.0/default/Light.h @@ -16,7 +16,7 @@ #ifndef ANDROID_HARDWARE_TESTS_EXTENSION_LIGHT_V2_0_LIGHT_H #define ANDROID_HARDWARE_TESTS_EXTENSION_LIGHT_V2_0_LIGHT_H -#include +#include #include #include @@ -28,26 +28,25 @@ namespace light { namespace V2_0 { namespace implementation { -using ::android::hardware::tests::extension::light::V2_0::ExtLightState; -using ::android::hardware::tests::extension::light::V2_0::IExtLight; -using ::android::hardware::light::V2_0::ILight; -using ::android::hardware::light::V2_0::LightState; +using ::android::hardware::tests::extension::light::V2_0::LightState; +using ::android::hardware::tests::extension::light::V2_0::ILight; +// If using a minor version upgrade, we could just reference these as +// V2_0::LightState vs. V2_1::LightState, but this makes things easier. +using OldLightState = ::android::hardware::light::V2_0::LightState; using ::android::hardware::light::V2_0::Status; using ::android::hardware::light::V2_0::Type; using ::android::hardware::Return; using ::android::hardware::Void; using ::android::hardware::hidl_vec; -using ::android::hardware::hidl_string; using ::android::sp; -struct Light : public IExtLight { +struct Light : public ILight { // Methods from ::android::hardware::light::V2_0::ILight follow. - Return setLight(Type type, const LightState& state) override; + Return setLight(Type type, const OldLightState& state) override; Return getSupportedTypes(getSupportedTypes_cb _hidl_cb) override; // Methods from ::android::hardware::example::extension::light::V2_0::ILight follow. - Return setExtLight(Type type, const ExtLightState& state) override; - + Return setLightExt(Type type, const LightState& state) override; }; } // namespace implementation diff --git a/tests/extension/light/2.0/types.hal b/tests/extension/light/2.0/types.hal index 1b094796db..c05e099f2a 100644 --- a/tests/extension/light/2.0/types.hal +++ b/tests/extension/light/2.0/types.hal @@ -16,7 +16,11 @@ package android.hardware.tests.extension.light@2.0; -import android.hardware.light@2.0; +// If importing something from the same package, the preference is: +// import @2.0::Foo. However, since this isn't a minor version upgrade +// we have to use the fully-qualified names here. +import android.hardware.light@2.0::Brightness; +import android.hardware.light@2.0::LightState; enum Default : int32_t { // for calls to setLight from the framework that don't know about this @@ -28,7 +32,7 @@ enum Default : int32_t { * One possibility is renaming an old type. Another possibility is taking * advantages of the different namespaces. */ -enum ExtBrightness : Brightness { +enum Brightness : android.hardware.light@2.0::Brightness { /** * Say we're really going to use the phone as a heater. */ @@ -43,10 +47,10 @@ enum ExtBrightness : Brightness { /** * Structs can't inherit eachother in hidl. Use composition instead. In this * case, I won't use inheritence because I want to replace Brightness with - * ExtBrightness. + * the new enumeration. */ -struct ExtLightState { - LightState state; +struct LightState { + android.hardware.light@2.0::LightState state; /** * This is the secret sauce that will really make this extension shine. @@ -64,5 +68,5 @@ struct ExtLightState { /** * Include new values. */ - ExtBrightness brightness; + Brightness brightness; };