From bf6e86d9f4baeff771752c20429909dfe27adc09 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 24 Sep 2026 02:26:14 +0100 Subject: [PATCH] [rtttl] Play a configured song from static storage without copying it Rtttl::play took the song by value and kept it in a std::string, so every rtttl.play with a constant song materialized a heap copy of the whole song, on ESP8266 copied out of PROGMEM, just to parse it. The parser now reads through a pointer and a 16 bit length. The std::string overload keeps its owned copy for lambdas and external callers and is unchanged. Generated code passes a constant as StaticSong, which points at the literal, a PROGMEM string on ESP8266 read with progmem_read_byte, so no copy is made per play. A pointer overload of play is deliberately avoided so a caller with a stack buffer cannot end up dangling. The small members are grouped so the object does not grow; the validator caps a configured song at what the length field holds. A host unit test covers header parsing, note parsing from static storage, the owned path, the busy check and the length clamp. --- esphome/components/rtttl/__init__.py | 13 ++- esphome/components/rtttl/rtttl.cpp | 106 ++++++++++++++---- esphome/components/rtttl/rtttl.h | 65 ++++++++--- tests/components/rtttl/test_rtttl_parser.cpp | 109 +++++++++++++++++++ 4 files changed, 253 insertions(+), 40 deletions(-) create mode 100644 tests/components/rtttl/test_rtttl_parser.cpp diff --git a/esphome/components/rtttl/__init__.py b/esphome/components/rtttl/__init__.py index 4f4d17b441e..a7cc3b3ee47 100644 --- a/esphome/components/rtttl/__init__.py +++ b/esphome/components/rtttl/__init__.py @@ -13,6 +13,8 @@ _LOGGER = logging.getLogger(__name__) CODEOWNERS = ["@glmnet", "@ximex"] CONF_RTTTL = "rtttl" +# The player keeps the song length in 16 bits +MAX_SONG_LENGTH = 65535 CONF_ON_FINISHED_PLAYBACK = "on_finished_playback" rtttl_ns = cg.esphome_ns.namespace("rtttl") @@ -96,16 +98,23 @@ async def to_code(config: ConfigType) -> None: await automation.build_callback_automations(var, config, _CALLBACK_AUTOMATIONS) +def _static_song(config: ConfigType, value: str) -> str: + # A constant song plays straight from static storage, flash on ESP8266, with no copy per play + return f"esphome::rtttl::StaticSong{{{cg.FlashStringLiteral(value)}}}" + + automation.register_apply_action( "rtttl.play", cv.maybe_simple_value( { cv.GenerateID(CONF_ID): cv.use_id(Rtttl), - cv.Required(CONF_RTTTL): cv.templatable(cv.string), + cv.Required(CONF_RTTTL): cv.templatable( + cv.All(cv.string, cv.Length(max=MAX_SONG_LENGTH)) + ), }, key=CONF_RTTTL, ), - automation.ApplyField(CONF_RTTTL, "play", cg.std_string), + automation.ApplyField(CONF_RTTTL, "play", cg.std_string, const_fn=_static_song), ) automation.register_apply_action( diff --git a/esphome/components/rtttl/rtttl.cpp b/esphome/components/rtttl/rtttl.cpp index a5f8567c9da..50ef5803cc4 100644 --- a/esphome/components/rtttl/rtttl.cpp +++ b/esphome/components/rtttl/rtttl.cpp @@ -1,5 +1,8 @@ #include "rtttl.h" +#include #include +#include +#include #include "esphome/core/hal.h" #include "esphome/core/log.h" #include "esphome/core/progmem.h" @@ -147,14 +150,14 @@ void Rtttl::loop() { #endif // USE_SPEAKER // Align to note: most rtttl's out there does not add any space after the ',' separator but just in case - while (this->position_ < this->rtttl_.length()) { - char c = this->rtttl_[this->position_]; + while (this->position_ < this->song_len_) { + char c = this->song_at_(this->position_); if (c != ',' && c != ' ') break; this->position_++; } - if (this->position_ >= this->rtttl_.length()) { + if (this->position_ >= this->song_len_) { this->finish_(); return; } @@ -169,12 +172,12 @@ void Rtttl::loop() { this->note_duration_ = this->wholenote_duration_ / this->default_note_denominator_; } - uint8_t note_index_in_octave = note_index_from_char(this->rtttl_[this->position_]); + uint8_t note_index_in_octave = note_index_from_char(this->song_at_(this->position_)); this->position_++; // Now, get optional '#' sharp - if (this->rtttl_[this->position_] == '#') { + if (this->song_at_(this->position_) == '#') { note_index_in_octave++; this->position_++; } @@ -192,7 +195,7 @@ void Rtttl::loop() { } // Now, get optional '.' dotted note - if (this->rtttl_[this->position_] == '.') { + if (this->song_at_(this->position_) == '.') { this->note_duration_ += this->note_duration_ / 2; // Duration +50% this->position_++; } @@ -264,15 +267,68 @@ void Rtttl::loop() { } void Rtttl::play(std::string rtttl) { - if (this->state_ != State::STOPPED && this->state_ != State::STOPPING) { - size_t pos = this->rtttl_.find(':'); - size_t len = (pos != std::string::npos) ? pos : this->rtttl_.length(); - ESP_LOGW(TAG, "Already playing: %.*s", (int) len, this->rtttl_.c_str()); + if (this->is_busy_()) { return; } + this->owned_song_ = std::move(rtttl); + this->set_song_(this->owned_song_.data(), this->owned_song_.size()); + this->start_(); +} - this->rtttl_ = std::move(rtttl); +void Rtttl::play(StaticSong song) { + if (this->is_busy_()) { + return; + } + // Release a previously owned song; the static one needs no storage. + std::string().swap(this->owned_song_); + const char *data = reinterpret_cast(song.rtttl); + this->set_song_(data, ESPHOME_strlen_P(data)); + this->start_(); +} +void Rtttl::set_song_(const char *song, size_t len) { + this->song_ = song; + // Anything past what the 16 bit length holds is dropped; the validator keeps configured songs shorter. + this->song_len_ = static_cast(std::min(len, std::numeric_limits::max())); +} + +bool Rtttl::is_busy_() const { + if (this->state_ == State::STOPPED || this->state_ == State::STOPPING) { + return false; + } + char name[SONG_NAME_LENGTH_LIMIT + 1]; + this->song_name_(name, sizeof(name), this->song_find_(':', 0, this->song_len_)); + ESP_LOGW(TAG, "Already playing: %s", name); + return true; +} + +size_t Rtttl::song_find_(char c, size_t from, size_t end) const { + for (size_t pos = from; pos < end; pos++) { + if (this->song_at_(pos) == c) + return pos; + } + return end; +} + +size_t Rtttl::song_find_control_(char key, size_t from, size_t end) const { + size_t pos = from; + while ((pos = this->song_find_(key, pos, end)) < end) { + if (this->song_at_(pos + 1) == '=') + return pos; + pos++; + } + return end; +} + +void Rtttl::song_name_(char *buf, size_t size, size_t name_len) const { + size_t len = std::min(name_len, size - 1); + for (size_t pos = 0; pos < len; pos++) { + buf[pos] = this->song_at_(pos); + } + buf[len] = '\0'; +} + +void Rtttl::start_() { this->default_note_denominator_ = DEFAULT_NOTE_DENOMINATOR; this->default_octave_ = DEFAULT_OCTAVE; this->note_duration_ = 0; @@ -281,9 +337,9 @@ void Rtttl::play(std::string rtttl) { uint16_t num; // Used for: default note-denominator, default octave, BPM // Get name - this->position_ = this->rtttl_.find(':'); + this->position_ = this->song_find_(':', 0, this->song_len_); - if (this->position_ == std::string::npos) { + if (this->position_ == this->song_len_) { ESP_LOGE(TAG, "Unable to determine name; missing ':'"); return; } @@ -292,18 +348,20 @@ void Rtttl::play(std::string rtttl) { static_cast(SONG_NAME_LENGTH_LIMIT)); return; } - ESP_LOGD(TAG, "Playing song %.*s", (int) this->position_, this->rtttl_.c_str()); + char name[SONG_NAME_LENGTH_LIMIT + 1]; + this->song_name_(name, sizeof(name), this->position_); + ESP_LOGD(TAG, "Playing song %s", name); size_t name_end_position = this->position_; - size_t control_end = this->rtttl_.find(':', name_end_position + 1); - if (control_end == std::string::npos) { + size_t control_end = this->song_find_(':', name_end_position + 1, this->song_len_); + if (control_end == this->song_len_) { ESP_LOGE(TAG, "Missing second ':'"); return; } // Get default duration - size_t pos = this->rtttl_.find("d=", name_end_position); - if (pos == std::string::npos || pos >= control_end) { + size_t pos = this->song_find_control_('d', name_end_position, control_end); + if (pos == control_end) { ESP_LOGW(TAG, "Missing 'd='; use default duration %d", this->default_note_denominator_); } else { this->position_ = pos + 2; @@ -317,8 +375,8 @@ void Rtttl::play(std::string rtttl) { } // Get default octave - pos = this->rtttl_.find("o=", name_end_position); - if (pos == std::string::npos || pos >= control_end) { + pos = this->song_find_control_('o', name_end_position, control_end); + if (pos == control_end) { ESP_LOGW(TAG, "Missing 'o='; use default octave %d", this->default_octave_); } else { this->position_ = pos + 2; @@ -332,8 +390,8 @@ void Rtttl::play(std::string rtttl) { } // Get BPM - pos = this->rtttl_.find("b=", name_end_position); - if (pos == std::string::npos || pos >= control_end) { + pos = this->song_find_control_('b', name_end_position, control_end); + if (pos == control_end) { ESP_LOGW(TAG, "Missing 'b='; use default BPM %d", bpm); } else { this->position_ = pos + 2; @@ -387,7 +445,7 @@ void Rtttl::stop() { } #endif // USE_SPEAKER - this->position_ = this->rtttl_.length(); + this->position_ = this->song_len_; this->note_duration_ = 0; } @@ -411,7 +469,7 @@ void Rtttl::finish_() { #endif // USE_SPEAKER // Ensure no more notes are played in case finish_() is called for an error. - this->position_ = this->rtttl_.length(); + this->position_ = this->song_len_; this->note_duration_ = 0; } diff --git a/esphome/components/rtttl/rtttl.h b/esphome/components/rtttl/rtttl.h index 7a32b79b177..ade2dcd3205 100644 --- a/esphome/components/rtttl/rtttl.h +++ b/esphome/components/rtttl/rtttl.h @@ -3,6 +3,7 @@ #include "esphome/core/component.h" #include "esphome/core/defines.h" #include "esphome/core/helpers.h" +#include "esphome/core/progmem.h" #ifdef USE_OUTPUT #include "esphome/components/output/float_output.h" @@ -14,6 +15,10 @@ namespace esphome::rtttl { +namespace testing { +class RtttlParserAccess; +} // namespace testing + inline constexpr uint8_t DEFAULT_NOTE_DENOMINATOR = 4; // Default note-denominator (quarter note) inline constexpr uint8_t DEFAULT_OCTAVE = 6; // Default octave for a note (see: `MIN_OCTAVE`, `MAX_OCTAVE` in `rtttl.cpp`) @@ -26,6 +31,11 @@ enum class State : uint8_t { STOPPING, }; +/// A song in storage that outlives playback, such as a literal from generated code; PROGMEM on ESP8266. +struct StaticSong { + ProgmemStr rtttl; +}; + class Rtttl final : public Component { public: #ifdef USE_OUTPUT @@ -38,7 +48,10 @@ class Rtttl final : public Component { void dump_config() override; void loop() override; + /// Play a song; the string is copied so it may come from anywhere. void play(std::string rtttl); + /// Play a song from static storage without copying it. + void play(StaticSong song); void stop(); float get_gain() { return this->gain_; } @@ -55,11 +68,29 @@ class Rtttl final : public Component { protected: inline uint16_t get_integer_() { uint16_t ret = 0; - while (isdigit(this->rtttl_[this->position_])) { - ret = (ret * 10) + (this->rtttl_[this->position_++] - '0'); + while (isdigit(this->song_at_(this->position_))) { + ret = (ret * 10) + (this->song_at_(this->position_++) - '0'); } return ret; } + /// The byte of the song at pos, or NUL past its end; the read is flash safe for a PROGMEM song on ESP8266. + char song_at_(size_t pos) const { + if (pos >= this->song_len_) + return '\0'; + return static_cast(progmem_read_byte(reinterpret_cast(this->song_) + pos)); + } + /// Index of the first `c` in [from, end), or `end` when absent. + size_t song_find_(char c, size_t from, size_t end) const; + /// Index of `key` followed by '=' in [from, end), or `end` when absent. + size_t song_find_control_(char key, size_t from, size_t end) const; + /// Copies the first name_len bytes of the song, its name, into buf for logging. + void song_name_(char *buf, size_t size, size_t name_len) const; + /// Points the view at a song, clamped to what song_len_ can hold. + void set_song_(const char *song, size_t len); + /// Parses the header and starts playback of the song the view points at. + void start_(); + /// Logs and returns true when a song is still playing, so a new one must wait. + bool is_busy_() const; /** * @brief Finalizes the playback of the RTTTL string. * @@ -70,24 +101,28 @@ class Rtttl final : public Component { void finish_(); void set_state_(State state); - /// The RTTTL string to play. - std::string rtttl_; - /// The current position in the RTTTL string. - size_t position_{0}; - /// The default duration of a note (e.g. 4 for a quarter note). - uint8_t default_note_denominator_{DEFAULT_NOTE_DENOMINATOR}; - /// The default octave for a note. - uint8_t default_octave_{DEFAULT_OCTAVE}; - /// The duration of the current note in milliseconds. - uint16_t note_duration_{0}; - /// The duration of a whole note in milliseconds. - uint16_t wholenote_duration_; + /// The song being played; points at owned_song_ or at static storage. + const char *song_{nullptr}; + /// Backing store for play(std::string). + std::string owned_song_; /// The time in milliseconds since microcontroller boot when the last note was started. uint32_t last_note_start_time_; /// The frequency of the current note in Hz. uint32_t output_freq_{0}; /// The gain of the output. float gain_{0.6f}; + /// Length of the song in bytes; songs are a few hundred bytes, so 16 bits is ample. + uint16_t song_len_{0}; + /// The current position in the song. + uint16_t position_{0}; + /// The duration of the current note in milliseconds. + uint16_t note_duration_{0}; + /// The duration of a whole note in milliseconds. + uint16_t wholenote_duration_; + /// The default duration of a note (e.g. 4 for a quarter note). + uint8_t default_note_denominator_{DEFAULT_NOTE_DENOMINATOR}; + /// The default octave for a note. + uint8_t default_octave_{DEFAULT_OCTAVE}; /// The current state of the RTTTL player. State state_{State::STOPPED}; @@ -113,6 +148,8 @@ class Rtttl final : public Component { /// The callback to call when playback is finished. CallbackManager on_finished_playback_callback_; #endif + + friend class testing::RtttlParserAccess; }; } // namespace esphome::rtttl diff --git a/tests/components/rtttl/test_rtttl_parser.cpp b/tests/components/rtttl/test_rtttl_parser.cpp new file mode 100644 index 00000000000..3c706f29877 --- /dev/null +++ b/tests/components/rtttl/test_rtttl_parser.cpp @@ -0,0 +1,109 @@ +#include + +#include + +#include "esphome/components/rtttl/rtttl.h" + +namespace esphome::rtttl::testing { + +// Reads the parser state the tests check; Rtttl is final, so this is a friend rather than a subclass. +class RtttlParserAccess { + public: + static uint8_t denominator(const Rtttl &p) { return p.default_note_denominator_; } + static uint8_t octave(const Rtttl &p) { return p.default_octave_; } + static uint16_t wholenote(const Rtttl &p) { return p.wholenote_duration_; } + static uint16_t note_duration(const Rtttl &p) { return p.note_duration_; } + static uint32_t freq(const Rtttl &p) { return p.output_freq_; } + static size_t position(const Rtttl &p) { return p.position_; } + static char at(const Rtttl &p, size_t pos) { return p.song_at_(pos); } + static void set_running(Rtttl &p) { p.state_ = State::RUNNING; } +}; + +using Access = RtttlParserAccess; + +static const char SONG[] = "test:d=8,o=5,b=100:c"; + +TEST(RtttlParser, StaticSongParsesTheHeader) { + Rtttl player; + player.play(StaticSong{SONG}); + EXPECT_EQ(Access::denominator(player), 8); + EXPECT_EQ(Access::octave(player), 5); + EXPECT_EQ(Access::wholenote(player), 60 * 1000 * 4 / 100); + // Positioned on the first note after the second ':' + EXPECT_EQ(Access::position(player), 19u); + EXPECT_EQ(Access::at(player, Access::position(player)), 'c'); +} + +TEST(RtttlParser, OwnedSongParsesTheSameAndOutlivesTheCaller) { + Rtttl player; + { + std::string song(SONG); + player.play(song); + } + EXPECT_EQ(Access::denominator(player), 8); + EXPECT_EQ(Access::octave(player), 5); + EXPECT_EQ(Access::at(player, 0), 't'); + EXPECT_EQ(Access::at(player, Access::position(player)), 'c'); + EXPECT_EQ(Access::at(player, 100), '\0'); +} + +TEST(RtttlParser, ControlOrderDoesNotMatter) { + for (const char *song : {"a:b=100,o=5,d=8:c", "a:o=5,b=100,d=8:c", "a:d=8,b=100,o=5:c"}) { + Rtttl player; + player.play(StaticSong{song}); + EXPECT_EQ(Access::denominator(player), 8) << song; + EXPECT_EQ(Access::octave(player), 5) << song; + EXPECT_EQ(Access::wholenote(player), 2400) << song; + } +} + +TEST(RtttlParser, MissingControlsUseDefaults) { + Rtttl player; + player.play(StaticSong{"a::c"}); + EXPECT_EQ(Access::denominator(player), DEFAULT_NOTE_DENOMINATOR); + EXPECT_EQ(Access::octave(player), DEFAULT_OCTAVE); + EXPECT_EQ(Access::wholenote(player), 60 * 1000 * 4 / 63); +} + +TEST(RtttlParser, NotesReadFromStaticStorage) { + Rtttl player; + player.play(StaticSong{"t:d=4,o=5,b=120:8c,p"}); + Access::set_running(player); + player.loop(); + // C5 at 523 Hz for an eighth of a 2000 ms whole note + EXPECT_EQ(Access::freq(player), 523u); + EXPECT_EQ(Access::note_duration(player), 250); + player.loop(); + // A pause for a default quarter note + EXPECT_EQ(Access::freq(player), 0u); + EXPECT_EQ(Access::note_duration(player), 500); + player.loop(); + // End of the song + EXPECT_EQ(Access::at(player, Access::position(player)), '\0'); +} + +TEST(RtttlParser, RefusesANewSongWhilePlaying) { + Rtttl player; + player.play(StaticSong{"first:d=8:c"}); + Access::set_running(player); + player.play(std::string("second:d=16:c")); + EXPECT_EQ(Access::denominator(player), 8); + EXPECT_EQ(Access::at(player, 0), 'f'); +} + +TEST(RtttlParser, ClampsASongLongerThanTheLengthField) { + Rtttl player; + player.play(std::string(70000, 'a')); + EXPECT_EQ(Access::at(player, 65534), 'a'); + EXPECT_EQ(Access::at(player, 65535), '\0'); + EXPECT_EQ(Access::note_duration(player), 0); +} + +TEST(RtttlParser, RejectsASongWithoutAName) { + Rtttl player; + player.play(StaticSong{"no colon here"}); + EXPECT_EQ(Access::note_duration(player), 0); + EXPECT_EQ(Access::position(player), 13u); +} + +} // namespace esphome::rtttl::testing