From aab6fa344e069989ee030b2aac895c5045d21f36 Mon Sep 17 00:00:00 2001 From: pawelirh Date: Wed, 4 Sep 2024 13:12:15 +0000 Subject: [PATCH] Test apa102 --- .../include/panther_lights/apa102.hpp | 62 ++++++- panther_lights/src/apa102.cpp | 46 ++--- panther_lights/src/lights_driver_node.cpp | 4 +- panther_lights/test/test_apa102.cpp | 175 ++++++++++++++---- 4 files changed, 218 insertions(+), 69 deletions(-) diff --git a/panther_lights/include/panther_lights/apa102.hpp b/panther_lights/include/panther_lights/apa102.hpp index 12c644f2..801a0610 100644 --- a/panther_lights/include/panther_lights/apa102.hpp +++ b/panther_lights/include/panther_lights/apa102.hpp @@ -15,6 +15,11 @@ #ifndef PANTHER_LIGHTS_APA102_HPP_ #define PANTHER_LIGHTS_APA102_HPP_ +#include +#include +#include +#include + #include #include #include @@ -23,6 +28,50 @@ namespace panther_lights { +class SPIDeviceInterface +{ +public: + virtual ~SPIDeviceInterface() = default; + + /** + * @brief Open SPI device + * + * @param device Name of the device + */ + virtual int Open(const std::string & device) = 0; + + /** + * @brief Perform an I/O control operation on the device + * + * @param fd File descriptor + * @param request Request code + * @param arg Argument + */ + virtual int IOControl(int fd, unsigned long request, const void * arg) = 0; + + /** + * @brief Close the device + * + * @param fd File descriptor + */ + virtual int Close(int fd) = 0; + + using SharedPtr = std::shared_ptr; +}; + +class SPIDevice : public SPIDeviceInterface +{ +public: + int Open(const std::string & device) override { return ::open(device.c_str(), O_WRONLY); } + + int IOControl(int fd, unsigned long request, const void * arg) override + { + return ::ioctl(fd, request, arg); + } + + int Close(int fd) override { return ::close(fd); } +}; + class APA102Interface { public: @@ -40,12 +89,18 @@ class APA102Interface * * This class provides methods to control the APA102 LED panel, including setting the global * brightness, setting the LED panel based on a given frame. + * + * @param spi_device SPI Device object + * @param device_name name of the device + * @param speed Speed of the SPI communication + * @param cs_high Chip select high flag */ class APA102 : public APA102Interface { public: APA102( - const std::string & device, const std::uint32_t speed = 800000, const bool cs_high = false); + SPIDeviceInterface::SharedPtr spi_device, const std::string & device_name, + const std::uint32_t speed = 800000, const bool cs_high = false); ~APA102(); /** @@ -104,9 +159,10 @@ class APA102 : public APA102Interface static constexpr std::uint16_t kCorrGreen = 200; static constexpr std::uint16_t kCorrBlue = 62; - const int fd_; - const std::string device_; + SPIDeviceInterface::SharedPtr spi_device_; + const std::string device_name_; const std::uint32_t speed_; + const int file_descriptor_; }; } // namespace panther_lights diff --git a/panther_lights/src/apa102.cpp b/panther_lights/src/apa102.cpp index 77eb77ff..8a2a70aa 100644 --- a/panther_lights/src/apa102.cpp +++ b/panther_lights/src/apa102.cpp @@ -14,11 +14,6 @@ #include "panther_lights/apa102.hpp" -#include -#include -#include -#include - #include #include #include @@ -30,36 +25,41 @@ namespace panther_lights { -APA102::APA102(const std::string & device, const std::uint32_t speed, const bool cs_high) -: fd_(open(device.c_str(), O_WRONLY)), device_(device), speed_(speed) +APA102::APA102( + SPIDeviceInterface::SharedPtr spi_device, const std::string & device_name, + const std::uint32_t speed, const bool cs_high) +: spi_device_(spi_device), + device_name_(device_name), + speed_(speed), + file_descriptor_(spi_device->Open(device_name)) { - if (fd_ < 0) { - throw std::ios_base::failure("Failed to open " + device_ + "."); + if (file_descriptor_ < 0) { + throw std::ios_base::failure("Failed to open " + device_name_ + "."); } static std::uint8_t mode = cs_high ? SPI_MODE_3 : SPI_MODE_3 | SPI_CS_HIGH; - if (ioctl(fd_, SPI_IOC_WR_MODE32, &mode) == -1) { - close(fd_); - throw std::ios_base::failure("Failed to set mode for " + device_ + "."); + if (spi_device_->IOControl(file_descriptor_, SPI_IOC_WR_MODE32, &mode) == -1) { + spi_device_->Close(file_descriptor_); + throw std::ios_base::failure("Failed to set mode for " + device_name_ + "."); } - if (ioctl(fd_, SPI_IOC_WR_BITS_PER_WORD, &kBits) == -1) { - close(fd_); - throw std::ios_base::failure("Can't set bits per word for " + device_ + "."); + if (spi_device_->IOControl(file_descriptor_, SPI_IOC_WR_BITS_PER_WORD, &kBits) == -1) { + spi_device_->Close(file_descriptor_); + throw std::ios_base::failure("Can't set bits per word for " + device_name_ + "."); } - if (ioctl(fd_, SPI_IOC_WR_MAX_SPEED_HZ, &speed_) == -1) { - close(fd_); - throw std::ios_base::failure("Can't set speed for " + device_ + "."); + if (spi_device_->IOControl(file_descriptor_, SPI_IOC_WR_MAX_SPEED_HZ, &speed_) == -1) { + spi_device_->Close(file_descriptor_); + throw std::ios_base::failure("Can't set speed for " + device_name_ + "."); } } -APA102::~APA102() { close(fd_); } +APA102::~APA102() { spi_device_->Close(file_descriptor_); } void APA102::SetGlobalBrightness(const float brightness) { if (brightness < 0.0f || brightness > 1.0f) { - throw std::out_of_range("Brightness out of range <0.0,1.0>."); + throw std::out_of_range("Brightness out of range [0.0, 1.0]."); } SetGlobalBrightness(std::uint8_t(ceil(brightness * 31.0f))); } @@ -67,7 +67,7 @@ void APA102::SetGlobalBrightness(const float brightness) void APA102::SetGlobalBrightness(const std::uint8_t brightness) { if (brightness > 31) { - throw std::out_of_range("Brightness out of range <0,31>."); + throw std::out_of_range("Brightness out of range [0, 31]."); } global_brightness_ = std::uint16_t(brightness); } @@ -119,10 +119,10 @@ void APA102::SPISendBuffer(const std::vector & buffer) const tr.delay_usecs = 0; tr.bits_per_word = kBits; - const int ret = ioctl(fd_, SPI_IOC_MESSAGE(1), &tr); + const int ret = spi_device_->IOControl(file_descriptor_, SPI_IOC_MESSAGE(1), &tr); if (ret < 1) { - throw std::ios_base::failure("Failed to send data over SPI " + device_ + "."); + throw std::ios_base::failure("Failed to send data over SPI " + device_name_ + "."); } } diff --git a/panther_lights/src/lights_driver_node.cpp b/panther_lights/src/lights_driver_node.cpp index eefd6638..966a0730 100644 --- a/panther_lights/src/lights_driver_node.cpp +++ b/panther_lights/src/lights_driver_node.cpp @@ -44,8 +44,8 @@ LightsDriverNode::LightsDriverNode(const rclcpp::NodeOptions & options) led_control_granted_(false), led_control_pending_(false), initialization_attempt_(0), - channel_1_(std::make_shared("/dev/spiled-channel1")), - channel_2_(std::make_shared("/dev/spiled-channel2")), + channel_1_(std::make_shared(std::make_shared(), "/dev/spiled-channel1")), + channel_2_(std::make_shared(std::make_shared(), "/dev/spiled-channel2")), diagnostic_updater_(this) { RCLCPP_INFO(this->get_logger(), "Constructing node."); diff --git a/panther_lights/test/test_apa102.cpp b/panther_lights/test/test_apa102.cpp index d0f1e37d..a0307d14 100644 --- a/panther_lights/test/test_apa102.cpp +++ b/panther_lights/test/test_apa102.cpp @@ -12,14 +12,33 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "gtest/gtest.h" +#include +#include #include "panther_lights/apa102.hpp" +static constexpr char kMockDeviceName[] = "/dev/mocked_device"; +static constexpr int kStartFrame = 0x00; +static constexpr int kEndFrame = 0xFF; + +class MockSPIDevice : public panther_lights::SPIDeviceInterface +{ +public: + MOCK_METHOD(int, Open, (const std::string & device), (override)); + MOCK_METHOD(int, IOControl, (int fd, unsigned long request, const void * arg), (override)); + MOCK_METHOD(int, Close, (int fd), (override)); + + // Nice mock suppresses warnings about uninteresting calls + using NiceMock = testing::NiceMock; +}; + class APA102Wrapper : public panther_lights::APA102 { public: - APA102Wrapper(const std::string & device) : APA102(device) {} + APA102Wrapper(std::shared_ptr spi_device, const std::string & device_name) + : APA102(spi_device, device_name) + { + } std::vector RGBAFrameToBGRBuffer(const std::vector & frame) const { @@ -31,52 +50,121 @@ class APA102Wrapper : public panther_lights::APA102 class TestAPA102 : public testing::Test { protected: - TestAPA102() { apa102_ = std::make_unique("/dev/spidev0.0"); } - + TestAPA102(); ~TestAPA102() { apa102_.reset(); } + std::shared_ptr spi_device_; std::unique_ptr apa102_; }; -TEST_F(TestAPA102, PortsAvailable) +TestAPA102::TestAPA102() +{ + spi_device_ = std::make_shared(); + + apa102_ = std::make_unique(spi_device_, kMockDeviceName); +} + +TEST_F(TestAPA102, TestInitialization) { EXPECT_TRUE(spi_device_ != nullptr); } + +TEST(TestInitialization, InvalidDevices) +{ + auto spi_device = std::make_shared(); + + // Return -1 to simulate failed device opening + ON_CALL(*spi_device, Open(kMockDeviceName)).WillByDefault(testing::Return(-1)); + + EXPECT_THROW(std::make_unique(spi_device, kMockDeviceName); + , std::ios_base::failure); +} + +TEST(TestInitialization, SetModeFailure) +{ + auto spi_device = std::make_shared(); + + // Return -1 to simulate failed setting mode + ON_CALL(*spi_device, IOControl(testing::_, SPI_IOC_WR_MODE32, testing::_)) + .WillByDefault(testing::Return(-1)); + EXPECT_CALL(*spi_device, Close(testing::_)).Times(1); + + EXPECT_THROW(std::make_unique(spi_device, kMockDeviceName); + , std::ios_base::failure); +} + +TEST(TestInitialization, SetBitsFailure) { - EXPECT_NO_THROW({ panther_lights::APA102 channel_1_("/dev/spidev0.0"); }); - EXPECT_NO_THROW({ panther_lights::APA102 channel_2_("/dev/spidev0.1"); }); + auto spi_device = std::make_shared(); + + // Return -1 to simulate failed setting bits + ON_CALL(*spi_device, IOControl(testing::_, SPI_IOC_WR_BITS_PER_WORD, testing::_)) + .WillByDefault(testing::Return(-1)); + EXPECT_CALL(*spi_device, Close(testing::_)).Times(1); + + EXPECT_THROW(std::make_unique(spi_device, kMockDeviceName); + , std::ios_base::failure); +} + +TEST(TestInitialization, SetSpeedFailure) +{ + auto spi_device = std::make_shared(); + + // Return -1 to simulate failed setting speed + ON_CALL(*spi_device, IOControl(testing::_, SPI_IOC_WR_MAX_SPEED_HZ, testing::_)) + .WillByDefault(testing::Return(-1)); + EXPECT_CALL(*spi_device, Close(testing::_)).Times(1); + + EXPECT_THROW(std::make_unique(spi_device, kMockDeviceName); + , std::ios_base::failure); +} + +TEST_F(TestAPA102, SetGlobalBrightnessRatioNegative) +{ + const float brightness_ratio = -1.0; + EXPECT_THROW(apa102_->SetGlobalBrightness(brightness_ratio), std::out_of_range); } -TEST_F(TestAPA102, SetGlobalBrightnessFloat) +TEST_F(TestAPA102, SetGlobalBrightnessRatioTooHigh) { - EXPECT_NO_THROW(apa102_->SetGlobalBrightness(static_cast(0))); - EXPECT_EQ(apa102_->GetGlobalBrightness(), 0); + const float brightness_ratio = 1.1; + EXPECT_THROW(apa102_->SetGlobalBrightness(brightness_ratio), std::out_of_range); +} - EXPECT_NO_THROW(apa102_->SetGlobalBrightness(static_cast(0.001))); - EXPECT_EQ(apa102_->GetGlobalBrightness(), 1); +TEST_F(TestAPA102, SetGlobalBrightnessRatioValid) +{ + const float brightness_ratio = 0.5; - EXPECT_NO_THROW(apa102_->SetGlobalBrightness(static_cast(0.5))); + ASSERT_NO_THROW(apa102_->SetGlobalBrightness(brightness_ratio)); EXPECT_EQ(apa102_->GetGlobalBrightness(), 16); +} + +TEST_F(TestAPA102, SetGlobalBrightnessNegative) +{ + // Wrap around to 255 + const std::uint8_t brightness = -1; - EXPECT_NO_THROW(apa102_->SetGlobalBrightness(static_cast(0.999))); - EXPECT_EQ(apa102_->GetGlobalBrightness(), 31); + EXPECT_THROW(apa102_->SetGlobalBrightness(brightness), std::out_of_range); +} - EXPECT_NO_THROW(apa102_->SetGlobalBrightness(static_cast(1.0))); - EXPECT_EQ(apa102_->GetGlobalBrightness(), 31); +TEST_F(TestAPA102, SetGlobalBrightnessTooHigh) +{ + const std::uint8_t brightness = 32; - EXPECT_THROW(apa102_->SetGlobalBrightness(static_cast(-1.0)), std::out_of_range); - EXPECT_THROW(apa102_->SetGlobalBrightness(static_cast(1.1)), std::out_of_range); + EXPECT_THROW(apa102_->SetGlobalBrightness(brightness), std::out_of_range); } -TEST_F(TestAPA102, SetGlobalBrightnessUint8) +TEST_F(TestAPA102, SetGlobalBrightnessValid) { - EXPECT_NO_THROW(apa102_->SetGlobalBrightness(std::uint8_t(0))); - EXPECT_EQ(apa102_->GetGlobalBrightness(), 0); + const std::uint8_t brightness = 16; - EXPECT_NO_THROW(apa102_->SetGlobalBrightness(std::uint8_t(16))); + ASSERT_NO_THROW(apa102_->SetGlobalBrightness(brightness)); EXPECT_EQ(apa102_->GetGlobalBrightness(), 16); +} - EXPECT_NO_THROW(apa102_->SetGlobalBrightness(std::uint8_t(31))); - EXPECT_EQ(apa102_->GetGlobalBrightness(), 31); +TEST_F(TestAPA102, RGBAFrameToBGRBufferInvalidFrame) +{ + std::vector frame = { + 255, 128, 64}; // Valid frame requires 4 values to match RGBA format - EXPECT_THROW(apa102_->SetGlobalBrightness(std::uint8_t(32)), std::out_of_range); + EXPECT_THROW(apa102_->RGBAFrameToBGRBuffer(frame), std::runtime_error); } TEST_F(TestAPA102, RGBAFrameToBGRBuffer) @@ -86,23 +174,28 @@ TEST_F(TestAPA102, RGBAFrameToBGRBuffer) apa102_->SetGlobalBrightness(std::uint8_t(16)); auto buffer = apa102_->RGBAFrameToBGRBuffer(frame); - EXPECT_EQ(buffer.size(), static_cast(12)); - EXPECT_EQ(buffer[0], 0x00); // Init frame - EXPECT_EQ(buffer[1], 0x00); // Init frame - EXPECT_EQ(buffer[2], 0x00); // Init frame - EXPECT_EQ(buffer[3], 0x00); // Init frame - EXPECT_EQ(buffer[4], 0xEC); // brightness value based on the frame - EXPECT_EQ(buffer[5], 0x0F); // B component after color correction - EXPECT_EQ(buffer[6], 0x64); // G component after color correction - EXPECT_EQ(buffer[7], 0xFF); // R component after color correction - EXPECT_EQ(buffer[8], 0xFF); // End frame - EXPECT_EQ(buffer[9], 0xFF); // End frame - EXPECT_EQ(buffer[10], 0xFF); // End frame - EXPECT_EQ(buffer[11], 0xFF); // End frame + ASSERT_EQ(buffer.size(), static_cast(12)); + + // Verify start frame + for (int i = 0; i < 4; i++) { + EXPECT_EQ(buffer[i], kStartFrame); + } + // Verify end frame + for (int i = 8; i < 12; i++) { + EXPECT_EQ(buffer[i], kEndFrame); + } + + // Verify RGBA frame + EXPECT_EQ(buffer[4], 0xEC); // brightness value based on the frame + EXPECT_EQ(buffer[5], 0x0F); // B component after color correction + EXPECT_EQ(buffer[6], 0x64); // G component after color correction + EXPECT_EQ(buffer[7], 0xFF); // R component after color correction } int main(int argc, char ** argv) { - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); + testing::InitGoogleTest(&argc, argv); + + auto result = RUN_ALL_TESTS(); + return result; }