From e47b4a5c3b6582266209e7db8fe73df43636b5c8 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 21 Nov 2025 16:41:05 -0800 Subject: [PATCH] [wpilib] Color: Improve string support (#8403) Now rgb() and color constants are supported. Changed from constructor to fromString() factory function to enable directly returning color constant values. --- .../main/native/include/wpi/util/Color.hpp | 71 +++++++++++++------ wpilibc/src/main/python/semiwrap/Color.yml | 2 +- .../src/test/native/cpp/util/ColorTest.cpp | 32 +++++++-- .../src/main/java/org/wpilib/util/Color.java | 60 +++++++++++++--- .../test/java/org/wpilib/util/ColorTest.java | 44 ++++++++++-- 5 files changed, 167 insertions(+), 42 deletions(-) diff --git a/wpilibc/src/main/native/include/wpi/util/Color.hpp b/wpilibc/src/main/native/include/wpi/util/Color.hpp index 4e75c55ad0..338a565430 100644 --- a/wpilibc/src/main/native/include/wpi/util/Color.hpp +++ b/wpilibc/src/main/native/include/wpi/util/Color.hpp @@ -5,6 +5,7 @@ #pragma once #include +#include #include #include #include @@ -774,38 +775,64 @@ class Color { constexpr Color(int r, int g, int b) : Color(r / 255.0, g / 255.0, b / 255.0) {} + constexpr bool operator==(const Color&) const = default; + /** - * Constructs a Color from a hex string. + * Makes a Color from a string. * - * @param hexString a string of the format \#RRGGBB + * @param str a string of the format `#RRGGBB` or `rgb(R, G, B)` + * @return the Color * @throws std::invalid_argument if the hex string is invalid. */ - explicit constexpr Color(std::string_view hexString) { - if (hexString.length() != 7 || !hexString.starts_with("#") || - !wpi::util::isHexDigit(hexString[1]) || - !wpi::util::isHexDigit(hexString[2]) || - !wpi::util::isHexDigit(hexString[3]) || - !wpi::util::isHexDigit(hexString[4]) || - !wpi::util::isHexDigit(hexString[5]) || - !wpi::util::isHexDigit(hexString[6])) { + static constexpr Color FromString(std::string_view str) { + if (str.empty()) { throw std::invalid_argument( - fmt::format("Invalid hex string for Color \"{}\"", hexString)); + fmt::format("Invalid color string \"{}\"", str)); } - int r = wpi::util::hexDigitValue(hexString[1]) * 16 + - wpi::util::hexDigitValue(hexString[2]); - int g = wpi::util::hexDigitValue(hexString[3]) * 16 + - wpi::util::hexDigitValue(hexString[4]); - int b = wpi::util::hexDigitValue(hexString[5]) * 16 + - wpi::util::hexDigitValue(hexString[6]); + // #RRGGBB style + if (str[0] == '#') { + if (str.length() != 7 || !str.starts_with("#") || + !wpi::util::isHexDigit(str[1]) || !wpi::util::isHexDigit(str[2]) || + !wpi::util::isHexDigit(str[3]) || !wpi::util::isHexDigit(str[4]) || + !wpi::util::isHexDigit(str[5]) || !wpi::util::isHexDigit(str[6])) { + throw std::invalid_argument( + fmt::format("Invalid hex string \"{}\"", str)); + } - red = r / 255.0; - green = g / 255.0; - blue = b / 255.0; + int r = wpi::util::hexDigitValue(str[1]) * 16 + + wpi::util::hexDigitValue(str[2]); + int g = wpi::util::hexDigitValue(str[3]) * 16 + + wpi::util::hexDigitValue(str[4]); + int b = wpi::util::hexDigitValue(str[5]) * 16 + + wpi::util::hexDigitValue(str[6]); + + return Color(r, g, b); + } + + // RGB style + if (str.starts_with("rgb(") && str.ends_with(")")) { + auto [redStr, gbStr] = + wpi::util::split(wpi::util::slice(str, 4, str.length() - 1), ','); + auto [greenStr, blueStr] = wpi::util::split(gbStr, ','); + if (blueStr.empty()) { + throw std::invalid_argument( + fmt::format("Invalid RGB string \"{}\"", str)); + } + auto r = wpi::util::parse_integer(wpi::util::trim(redStr), 10); + auto g = wpi::util::parse_integer(wpi::util::trim(greenStr), 10); + auto b = wpi::util::parse_integer(wpi::util::trim(blueStr), 10); + if (!r || !g || !b) { + throw std::invalid_argument( + fmt::format("Invalid RGB string \"{}\"", str)); + } + return Color(*r, *g, *b); + } + + throw std::invalid_argument( + fmt::format("Invalid color string \"{}\"", str)); } - constexpr bool operator==(const Color&) const = default; - /** * Creates a Color from HSV values. * diff --git a/wpilibc/src/main/python/semiwrap/Color.yml b/wpilibc/src/main/python/semiwrap/Color.yml index f21d8d4127..508d71e6f7 100644 --- a/wpilibc/src/main/python/semiwrap/Color.yml +++ b/wpilibc/src/main/python/semiwrap/Color.yml @@ -168,7 +168,7 @@ classes: b: name: blue int, int, int: - std::string_view: + FromString: FromHSV: HexString: operator==: diff --git a/wpilibc/src/test/native/cpp/util/ColorTest.cpp b/wpilibc/src/test/native/cpp/util/ColorTest.cpp index 1c47e158eb..7174a5447e 100644 --- a/wpilibc/src/test/native/cpp/util/ColorTest.cpp +++ b/wpilibc/src/test/native/cpp/util/ColorTest.cpp @@ -43,21 +43,43 @@ TEST(ColorTest, ConstructFromInts) { EXPECT_NEAR(0.25, color.blue, 1e-2); } -TEST(ColorTest, ConstructFromHexString) { - constexpr wpi::Color color{"#FF8040"}; +TEST(ColorTest, FromHexString) { + constexpr wpi::Color color = wpi::Color::FromString("#FF8040"); EXPECT_NEAR(1.0, color.red, 1e-2); EXPECT_NEAR(0.5, color.green, 1e-2); EXPECT_NEAR(0.25, color.blue, 1e-2); // No leading # - EXPECT_THROW(wpi::Color{"112233"}, std::invalid_argument); + EXPECT_THROW(wpi::Color::FromString("112233"), std::invalid_argument); // Too long - EXPECT_THROW(wpi::Color{"#11223344"}, std::invalid_argument); + EXPECT_THROW(wpi::Color::FromString("#11223344"), std::invalid_argument); // Invalid hex characters - EXPECT_THROW(wpi::Color{"#$$$$$$"}, std::invalid_argument); + EXPECT_THROW(wpi::Color::FromString("#$$$$$$"), std::invalid_argument); +} + +TEST(ColorTest, FromRGBString) { + constexpr wpi::Color color = wpi::Color::FromString("rgb(255, 128, 64)"); + + EXPECT_NEAR(1.0, color.red, 1e-2); + EXPECT_NEAR(0.5, color.green, 1e-2); + EXPECT_NEAR(0.25, color.blue, 1e-2); + + // Missing rgb() + EXPECT_THROW(wpi::Color::FromString("255, 128, 64"), std::invalid_argument); + + // Too few components + EXPECT_THROW(wpi::Color::FromString("rgb(255, 128)"), std::invalid_argument); + + // Too many components + EXPECT_THROW(wpi::Color::FromString("rgb(255, 128, 64, 32)"), + std::invalid_argument); + + // Non-integer component + EXPECT_THROW(wpi::Color::FromString("rgb(255, abc, 64)"), + std::invalid_argument); } TEST(ColorTest, FromHSV) { diff --git a/wpilibj/src/main/java/org/wpilib/util/Color.java b/wpilibj/src/main/java/org/wpilib/util/Color.java index 6e54786a8e..b3113c4e0d 100644 --- a/wpilibj/src/main/java/org/wpilib/util/Color.java +++ b/wpilibj/src/main/java/org/wpilib/util/Color.java @@ -81,19 +81,61 @@ public class Color { } /** - * Constructs a Color from a hex string. + * Makes a Color from a string. * - * @param hexString a string of the format #RRGGBB - * @throws IllegalArgumentException if the hex string is invalid. + * @param str a string of the format #RRGGBB, rgb(R, G, B), or + * ConstantName + * @return the Color + * @throws IllegalArgumentException if the string is invalid. */ - public Color(String hexString) { - if (hexString.length() != 7 || !hexString.startsWith("#")) { - throw new IllegalArgumentException("Invalid hex string \"" + hexString + "\""); + public static Color fromString(String str) { + if (str == null || str.isBlank()) { + throw new IllegalArgumentException("Invalid color string \"" + str + "\""); } - this.red = Integer.valueOf(hexString.substring(1, 3), 16) / 255.0; - this.green = Integer.valueOf(hexString.substring(3, 5), 16) / 255.0; - this.blue = Integer.valueOf(hexString.substring(5, 7), 16) / 255.0; + // #RRGGBB style + if (str.charAt(0) == '#') { + if (str.length() != 7) { + throw new IllegalArgumentException("Invalid hex string \"" + str + "\""); + } + return new Color( + Integer.parseInt(str.substring(1, 3), 16), + Integer.parseInt(str.substring(3, 5), 16), + Integer.parseInt(str.substring(5, 7), 16)); + } + + // RGB style + if (str.startsWith("rgb(") && str.endsWith(")")) { + String[] components = str.substring(4, str.length() - 1).split(","); + if (components.length != 3) { + throw new IllegalArgumentException("Invalid RGB string \"" + str + "\""); + } + try { + return new Color( + Integer.parseInt(components[0].trim()), + Integer.parseInt(components[1].trim()), + Integer.parseInt(components[2].trim())); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid RGB string \"" + str + "\""); + } + } + + // try to parse as a named color by matching against k-prefixed static constants in the Color + // class + String search = str.startsWith("k") ? str : "k" + str; + for (var field : Color.class.getFields()) { + if (java.lang.reflect.Modifier.isStatic(field.getModifiers()) + && Color.class.isAssignableFrom(field.getType()) + && field.getName().equalsIgnoreCase(search)) { + try { + return (Color) field.get(null); + } catch (IllegalAccessException e) { + // Ignore and continue + } + } + } + + throw new IllegalArgumentException("Invalid color string \"" + str + "\""); } /** diff --git a/wpilibj/src/test/java/org/wpilib/util/ColorTest.java b/wpilibj/src/test/java/org/wpilib/util/ColorTest.java index 5e6a8c5af4..50d14e30aa 100644 --- a/wpilibj/src/test/java/org/wpilib/util/ColorTest.java +++ b/wpilibj/src/test/java/org/wpilib/util/ColorTest.java @@ -55,21 +55,55 @@ class ColorTest { } @Test - void testConstructFromHexString() { - var color = new Color("#FF8040"); + void testFromHexString() { + var color = Color.fromString("#FF8040"); assertEquals(1.0, color.red, 1e-2); assertEquals(0.5, color.green, 1e-2); assertEquals(0.25, color.blue, 1e-2); // No leading # - assertThrows(IllegalArgumentException.class, () -> new Color("112233")); + assertThrows(IllegalArgumentException.class, () -> Color.fromString("112233")); // Too long - assertThrows(IllegalArgumentException.class, () -> new Color("#11223344")); + assertThrows(IllegalArgumentException.class, () -> Color.fromString("#11223344")); // Invalid hex characters - assertThrows(IllegalArgumentException.class, () -> new Color("#$$$$$$")); + assertThrows(IllegalArgumentException.class, () -> Color.fromString("#$$$$$$")); + } + + @Test + void testFromRGBString() { + var color = Color.fromString("rgb(255,128,64)"); + + assertEquals(1.0, color.red, 1e-2); + assertEquals(0.5, color.green, 1e-2); + assertEquals(0.25, color.blue, 1e-2); + + // Wrong number of components + assertThrows(IllegalArgumentException.class, () -> Color.fromString("rgb(255,128)")); + + // Invalid integer + assertThrows(IllegalArgumentException.class, () -> Color.fromString("rgb(255,xx,64)")); + } + + @Test + void testFromConstantName() { + var color = Color.fromString("red"); + + assertEquals(1.0, color.red, 1e-2); + assertEquals(0.0, color.green, 1e-2); + assertEquals(0.0, color.blue, 1e-2); + + // with k prefix + color = Color.fromString("kRed"); + + assertEquals(1.0, color.red, 1e-2); + assertEquals(0.0, color.green, 1e-2); + assertEquals(0.0, color.blue, 1e-2); + + // Unknown name + assertThrows(IllegalArgumentException.class, () -> Color.fromString("unknowncolor")); } @Test