From addaab20eaa7cd3120b339efb30ebd54c5196fab Mon Sep 17 00:00:00 2001 From: Anton Swifton <swifton@google.com> Date: Mon, 5 Dec 2022 21:59:40 +0000 Subject: [PATCH] RGB keyboard: Add API for setting a zone color. Add a DBus call for setting a zone color and an API to RgbKeyboardManager. Bug: b:259589687 Test: ash_unittests Change-Id: I8079bfc7f54cceb105d7bc59c99607215d307208 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4064922 Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org> Commit-Queue: Anton Swifton <swifton@google.com> Cr-Commit-Position: refs/heads/main@{#1079452} --- ash/rgb_keyboard/rgb_keyboard_manager.cc | 37 +++++++++++++++ ash/rgb_keyboard/rgb_keyboard_manager.h | 2 + .../rgb_keyboard_manager_unittest.cc | 45 +++++++++++++++++++ .../dbus/rgbkbd/fake_rgbkbd_client.cc | 4 ++ .../dbus/rgbkbd/fake_rgbkbd_client.h | 9 ++++ .../components/dbus/rgbkbd/rgbkbd_client.cc | 17 +++++++ .../components/dbus/rgbkbd/rgbkbd_client.h | 2 + 7 files changed, 116 insertions(+) diff --git a/ash/rgb_keyboard/rgb_keyboard_manager.cc b/ash/rgb_keyboard/rgb_keyboard_manager.cc index 9e261eac3ce8c..79eac5e337592 100644 --- a/ash/rgb_keyboard/rgb_keyboard_manager.cc +++ b/ash/rgb_keyboard/rgb_keyboard_manager.cc @@ -60,6 +60,20 @@ rgbkbd::RgbKeyboardCapabilities RgbKeyboardManager::GetRgbKeyboardCapabilities() return capabilities_; } +int RgbKeyboardManager::GetZoneCount() { + switch (capabilities_) { + case rgbkbd::RgbKeyboardCapabilities::kIndividualKey: + return 5; + case rgbkbd::RgbKeyboardCapabilities::kFourZoneFortyLed: + case rgbkbd::RgbKeyboardCapabilities::kFourZoneTwelveLed: + case rgbkbd::RgbKeyboardCapabilities::kFourZoneFourLed: + return 4; + case rgbkbd::RgbKeyboardCapabilities::kNone: + LOG(ERROR) << "Attempted to get zone count for a non-RGB keyboard."; + return 0; + } +} + void RgbKeyboardManager::SetStaticBackgroundColor(uint8_t r, uint8_t g, uint8_t b) { @@ -80,6 +94,28 @@ void RgbKeyboardManager::SetStaticBackgroundColor(uint8_t r, RgbkbdClient::Get()->SetStaticBackgroundColor(r, g, b); } +void RgbKeyboardManager::SetZoneColor(int zone, + uint8_t r, + uint8_t g, + uint8_t b) { + DCHECK(RgbkbdClient::Get()); + if (zone < 0 || zone >= GetZoneCount()) { + LOG(ERROR) << "Attempted to set an invalid zone: " << zone; + return; + } + if (!IsRgbKeyboardSupported()) { + LOG(ERROR) + << "Attempted to set RGB keyboard zone color, but flag is disabled."; + return; + } + + VLOG(1) << "Setting RGB keyboard zone " << zone + << " color to R:" << static_cast<int>(r) + << " G:" << static_cast<int>(g) << " B:" << static_cast<int>(b); + // TODO(swifton): Add metrics. + RgbkbdClient::Get()->SetZoneColor(zone, r, g, b); +} + void RgbKeyboardManager::SetRainbowMode() { DCHECK(RgbkbdClient::Get()); background_type_ = BackgroundType::kStaticRainbow; @@ -165,6 +201,7 @@ void RgbKeyboardManager::InitializeRgbKeyboard() { SkColorGetG(background_color_), SkColorGetB(background_color_)); break; + // TODO(swifton): Add a case for zone colors. case BackgroundType::kStaticRainbow: SetRainbowMode(); break; diff --git a/ash/rgb_keyboard/rgb_keyboard_manager.h b/ash/rgb_keyboard/rgb_keyboard_manager.h index 3ac027ef8df2b..d5e585a91dd42 100644 --- a/ash/rgb_keyboard/rgb_keyboard_manager.h +++ b/ash/rgb_keyboard/rgb_keyboard_manager.h @@ -32,7 +32,9 @@ class ASH_EXPORT RgbKeyboardManager : public ImeControllerImpl::Observer, ~RgbKeyboardManager() override; rgbkbd::RgbKeyboardCapabilities GetRgbKeyboardCapabilities() const; + int GetZoneCount(); void SetStaticBackgroundColor(uint8_t r, uint8_t g, uint8_t b); + void SetZoneColor(int zone, uint8_t r, uint8_t g, uint8_t b); void SetRainbowMode(); void SetAnimationMode(rgbkbd::RgbAnimationMode mode); diff --git a/ash/rgb_keyboard/rgb_keyboard_manager_unittest.cc b/ash/rgb_keyboard/rgb_keyboard_manager_unittest.cc index 7a48b3b7e58f5..403a3b6f6fe8f 100644 --- a/ash/rgb_keyboard/rgb_keyboard_manager_unittest.cc +++ b/ash/rgb_keyboard/rgb_keyboard_manager_unittest.cc @@ -16,6 +16,7 @@ #include "chromeos/ash/components/dbus/rgbkbd/fake_rgbkbd_client.h" #include "chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/abseil-cpp/absl/types/optional.h" namespace ash { @@ -151,6 +152,27 @@ TEST_P(RgbChangeTypeHistogramEmittedTest, RgbChangeTypeHistogramEmitted) { 1); } +TEST_F(RgbKeyboardManagerTest, ZoneCountIsCorrect) { + InitializeManagerWithCapability( + rgbkbd::RgbKeyboardCapabilities::kIndividualKey); + EXPECT_EQ(5, manager_->GetZoneCount()); + + InitializeManagerWithCapability( + rgbkbd::RgbKeyboardCapabilities::kFourZoneFortyLed); + EXPECT_EQ(4, manager_->GetZoneCount()); + + InitializeManagerWithCapability( + rgbkbd::RgbKeyboardCapabilities::kFourZoneTwelveLed); + EXPECT_EQ(4, manager_->GetZoneCount()); + + InitializeManagerWithCapability( + rgbkbd::RgbKeyboardCapabilities::kFourZoneFourLed); + EXPECT_EQ(4, manager_->GetZoneCount()); + + InitializeManagerWithCapability(rgbkbd::RgbKeyboardCapabilities::kNone); + EXPECT_EQ(0, manager_->GetZoneCount()); +} + TEST_F(RgbKeyboardManagerTest, SetStaticRgbValues) { const uint8_t expected_r = 1; const uint8_t expected_g = 2; @@ -164,6 +186,29 @@ TEST_F(RgbKeyboardManagerTest, SetStaticRgbValues) { EXPECT_EQ(expected_b, std::get<2>(rgb_values)); } +TEST_F(RgbKeyboardManagerTest, SetZoneRgbValues) { + const int zone_1 = 0; + const uint8_t expected_r_1 = 1; + const uint8_t expected_g_1 = 2; + const uint8_t expected_b_1 = 3; + + const int zone_2 = 3; + const uint8_t expected_r_2 = 4; + const uint8_t expected_g_2 = 5; + const uint8_t expected_b_2 = 6; + + manager_->SetZoneColor(zone_1, expected_r_1, expected_g_1, expected_b_1); + manager_->SetZoneColor(zone_2, expected_r_2, expected_g_2, expected_b_2); + + auto zone_colors = client_->get_zone_colors(); + + EXPECT_EQ(2u, zone_colors.size()); + EXPECT_EQ(zone_colors[zone_1], + std::make_tuple(expected_r_1, expected_g_1, expected_b_1)); + EXPECT_EQ(zone_colors[zone_2], + std::make_tuple(expected_r_2, expected_g_2, expected_b_2)); +} + TEST_F(RgbKeyboardManagerTest, SetRainbowMode) { EXPECT_FALSE(client_->is_rainbow_mode_set()); diff --git a/chromeos/ash/components/dbus/rgbkbd/fake_rgbkbd_client.cc b/chromeos/ash/components/dbus/rgbkbd/fake_rgbkbd_client.cc index 400965ca433c8..a68c0e74e3e3b 100644 --- a/chromeos/ash/components/dbus/rgbkbd/fake_rgbkbd_client.cc +++ b/chromeos/ash/components/dbus/rgbkbd/fake_rgbkbd_client.cc @@ -32,6 +32,10 @@ void FakeRgbkbdClient::ResetStoredRgbColors() { rgb_color_ = std::make_tuple(0u, 0u, 0u); } +void FakeRgbkbdClient::SetZoneColor(int zone, uint8_t r, uint8_t g, uint8_t b) { + zone_colors_[zone] = std::make_tuple(r, g, b); +} + void FakeRgbkbdClient::SetRainbowMode() { is_rainbow_mode_set_ = true; ResetStoredRgbColors(); diff --git a/chromeos/ash/components/dbus/rgbkbd/fake_rgbkbd_client.h b/chromeos/ash/components/dbus/rgbkbd/fake_rgbkbd_client.h index 35814170c8268..64eb1a51872ea 100644 --- a/chromeos/ash/components/dbus/rgbkbd/fake_rgbkbd_client.h +++ b/chromeos/ash/components/dbus/rgbkbd/fake_rgbkbd_client.h @@ -8,7 +8,9 @@ #include <stdint.h> #include "base/component_export.h" +#include "base/containers/flat_map.h" #include "chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.h" +#include "third_party/abseil-cpp/absl/types/optional.h" #include "third_party/cros_system_api/dbus/rgbkbd/dbus-constants.h" namespace ash { @@ -29,6 +31,8 @@ class COMPONENT_EXPORT(RGBKBD) FakeRgbkbdClient : public RgbkbdClient { void SetStaticBackgroundColor(uint8_t r, uint8_t g, uint8_t b) override; + void SetZoneColor(int zone, uint8_t r, uint8_t g, uint8_t b) override; + void SetRainbowMode() override; void SetAnimationMode(rgbkbd::RgbAnimationMode mode) override; @@ -49,6 +53,10 @@ class COMPONENT_EXPORT(RGBKBD) FakeRgbkbdClient : public RgbkbdClient { const RgbColor& recently_sent_rgb() const { return rgb_color_; } + const base::flat_map<int, RgbColor>& get_zone_colors() const { + return zone_colors_; + } + void attempt_run_rgb_keyboard_capabilities_callback() { if (callback_.is_null() || !should_run_callback_) return; @@ -69,6 +77,7 @@ class COMPONENT_EXPORT(RGBKBD) FakeRgbkbdClient : public RgbkbdClient { bool caps_lock_state_ = false; bool is_rainbow_mode_set_ = false; RgbColor rgb_color_; + base::flat_map<int, RgbColor> zone_colors_; int animation_mode_call_count_ = 0; GetRgbKeyboardCapabilitiesCallback callback_; diff --git a/chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.cc b/chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.cc index 3287f881d8c1b..d031b77ecd14d 100644 --- a/chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.cc +++ b/chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.cc @@ -68,6 +68,23 @@ class RgbkbdClientImpl : public RgbkbdClient { base::DoNothing()); } + void SetZoneColor(int zone, uint8_t r, uint8_t g, uint8_t b) override { + VLOG(1) << "rgbkbd: SetZoneColor Zone: " << zone + << "R: " << static_cast<int>(r) << "G: " << static_cast<int>(g) + << "B: " << static_cast<int>(b); + dbus::MethodCall method_call(rgbkbd::kRgbkbdServiceName, + rgbkbd::kSetZoneColor); + dbus::MessageWriter writer(&method_call); + writer.AppendInt32(zone); + writer.AppendByte(r); + writer.AppendByte(g); + writer.AppendByte(b); + CHECK(rgbkbd_proxy_); + rgbkbd_proxy_->CallMethod(&method_call, + dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, + base::DoNothing()); + } + void SetRainbowMode() override { VLOG(1) << "rgbkbd: SetRainbowMode"; dbus::MethodCall method_call(rgbkbd::kRgbkbdServiceName, diff --git a/chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.h b/chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.h index dcc8869735aaf..de80d6c118e1a 100644 --- a/chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.h +++ b/chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.h @@ -57,6 +57,8 @@ class COMPONENT_EXPORT(RGBKBD_CLIENT) RgbkbdClient { virtual void SetStaticBackgroundColor(uint8_t r, uint8_t g, uint8_t b) = 0; + virtual void SetZoneColor(int zone, uint8_t r, uint8_t g, uint8_t b) = 0; + virtual void SetRainbowMode() = 0; virtual void SetAnimationMode(rgbkbd::RgbAnimationMode mode) = 0;