[pixoo] Fix clang-tidy warnings and address review feedback

Move build_packet/pad_unused to file-local helpers, round brightness
with lroundf, free the draw buffer on frame-buffer alloc failure, and
pad the final DMA chunk when exactly the static packet length remains.
This commit is contained in:
Jesse Hills
2026-06-15 20:06:19 +12:00
parent c0d98b50ac
commit 962b65a46d
2 changed files with 25 additions and 23 deletions

View File

@@ -2,6 +2,7 @@
#include "esphome/core/log.h"
#include <cmath>
#include <cstring>
#include <utility>
@@ -9,9 +10,19 @@ namespace esphome::pixoo {
static const char *const TAG = "pixoo";
float Pixoo::get_setup_priority() const { return setup_priority::PROCESSOR; }
// Divoom LED-board packet protocol.
static constexpr uint8_t PACKET_HEAD = 0xAA;
static constexpr uint8_t PACKET_TAIL = 0xBB;
static constexpr uint8_t CMD_DATA = 0x00;
static constexpr uint8_t CMD_LIGHT = 0x01;
static constexpr uint8_t CMD_UNUSED = 0x21;
static constexpr uint8_t CMD_SET_RGB_IOUT = 0x22;
static constexpr size_t PACKET_HEADER_LEN = 4; // head + len(2) + cmd
static constexpr size_t PACKET_STATIC_LEN = 5; // header + tail
static constexpr uint8_t DEFAULT_IOUT = 75; // per-channel LED current / white balance default
size_t Pixoo::build_packet_(uint8_t *buf, uint8_t cmd, const uint8_t *data, uint16_t len) {
// Pack a `0xAA len cmd data 0xBB` packet into buf; returns the packet length.
static inline size_t build_packet(uint8_t *buf, uint8_t cmd, const uint8_t *data, uint16_t len) {
buf[0] = PACKET_HEAD;
buf[1] = static_cast<uint8_t>(len & 0xFF);
buf[2] = static_cast<uint8_t>((len >> 8) & 0xFF);
@@ -22,7 +33,8 @@ size_t Pixoo::build_packet_(uint8_t *buf, uint8_t cmd, const uint8_t *data, uint
return len + PACKET_STATIC_LEN;
}
void Pixoo::pad_unused_(uint8_t *buf, size_t total) {
// Fill `total` bytes at buf with a single UNUSED padding packet.
static inline void pad_unused(uint8_t *buf, size_t total) {
const uint16_t len = static_cast<uint16_t>(total - PACKET_STATIC_LEN);
buf[0] = PACKET_HEAD;
buf[1] = static_cast<uint8_t>(len & 0xFF);
@@ -31,6 +43,8 @@ void Pixoo::pad_unused_(uint8_t *buf, size_t total) {
buf[total - 1] = PACKET_TAIL;
}
float Pixoo::get_setup_priority() const { return setup_priority::PROCESSOR; }
void Pixoo::setup() {
const uint32_t num_pixels = static_cast<uint32_t>(this->model_) * this->model_;
this->data_size_ = num_pixels * 3;
@@ -47,6 +61,7 @@ void Pixoo::setup() {
RAMAllocator<uint8_t> allocator(RAMAllocator<uint8_t>::ALLOC_INTERNAL);
this->frame_buffer_ = allocator.allocate(this->frame_size_);
if (this->frame_buffer_ == nullptr) {
this->buffer_.free();
this->mark_failed(LOG_STR("Failed to allocate frame buffer"));
return;
}
@@ -57,7 +72,7 @@ void Pixoo::setup() {
this->frame_buffer_[2] = static_cast<uint8_t>((this->data_size_ >> 8) & 0xFF);
this->frame_buffer_[3] = CMD_DATA;
this->frame_buffer_[PACKET_HEADER_LEN + this->data_size_] = PACKET_TAIL;
this->pad_unused_(this->frame_buffer_ + this->data_size_ + PACKET_STATIC_LEN, DMA_CHUNK);
pad_unused(this->frame_buffer_ + this->data_size_ + PACKET_STATIC_LEN, DMA_CHUNK);
this->spi_setup();
@@ -75,16 +90,16 @@ void Pixoo::setup() {
void Pixoo::send_command_(uint8_t cmd, const uint8_t *data, uint16_t len) {
std::memset(this->cmd_buffer_, 0, DMA_CHUNK);
const size_t used = build_packet_(this->cmd_buffer_, cmd, data, len);
if (DMA_CHUNK - used > PACKET_STATIC_LEN)
pad_unused_(this->cmd_buffer_ + used, DMA_CHUNK - used);
const size_t used = build_packet(this->cmd_buffer_, cmd, data, len);
if (DMA_CHUNK - used >= PACKET_STATIC_LEN)
pad_unused(this->cmd_buffer_ + used, DMA_CHUNK - used);
this->enable();
this->write_array(this->cmd_buffer_, DMA_CHUNK);
this->disable();
}
void Pixoo::set_panel_brightness(float brightness) {
const uint8_t pct = static_cast<uint8_t>(clamp(brightness, 0.0f, 1.0f) * 100.0f + 0.5f);
const uint8_t pct = static_cast<uint8_t>(lroundf(clamp(brightness, 0.0f, 1.0f) * 100.0f));
this->send_command_(CMD_LIGHT, &pct, 1);
}

View File

@@ -49,22 +49,9 @@ class Pixoo : public Display,
void set_pixel_(uint32_t index, Color color);
void send_command_(uint8_t cmd, const uint8_t *data, uint16_t len);
// Pack a `0xAA len cmd data 0xBB` packet into buf; returns the packet length.
static size_t build_packet_(uint8_t *buf, uint8_t cmd, const uint8_t *data, uint16_t len);
// Fill `total` bytes at buf with a single UNUSED padding packet.
static void pad_unused_(uint8_t *buf, size_t total);
// Divoom LED-board packet protocol constants.
static constexpr uint8_t PACKET_HEAD = 0xAA;
static constexpr uint8_t PACKET_TAIL = 0xBB;
static constexpr uint8_t CMD_DATA = 0x00;
static constexpr uint8_t CMD_LIGHT = 0x01;
static constexpr uint8_t CMD_UNUSED = 0x21;
static constexpr uint8_t CMD_SET_RGB_IOUT = 0x22;
static constexpr size_t PACKET_HEADER_LEN = 4; // head + len(2) + cmd
static constexpr size_t PACKET_STATIC_LEN = 5; // header + tail
static constexpr size_t DMA_CHUNK = 240; // LED board DMA granularity
static constexpr uint8_t DEFAULT_IOUT = 75; // per-channel LED current / white balance default
// Size of the LED board's SPI DMA chunk; the command scratch buffer is one chunk.
static constexpr size_t DMA_CHUNK = 240;
PixooModel model_;