Add missing list attribute

[heavyrubberslave]

Summary by CodeRabbit

• New Features

  • Added baud rate and mode configuration options.
  • Added a list-based attribute type for enumerated configuration values.

• Refactor

  • Standardized output interface method naming and updated all protocol emission paths accordingly.

• Chores

  • Bumped package version (patch release).

[coderabbitai]

📝 Walkthrough

Walkthrough

Renamed the output API from send() to write() across the protocol abstraction and transport; added a public ListAttribute<T> template for attributes with fixed option sets (int or string); updated example to register baud and mode attributes and bumped package version.

Changes

Cohort / File(s)	Summary
Protocol & Attributes src/SlvCtrlProtocol.h	Renamed ISlvCtrlOut::send(...) → write(...); updated all attribute describe/writeValue and protocol response emission to use write. Added public template ListAttribute<T> supporting int32_t and const char* option lists, parsing, and option rendering.
Transport Implementation src/SlvCtrlArduinoSerialCommandsTransport.h	Updated concrete out implementation to replace send(...) overrides with write(...) overloads forwarding to the underlying serial print/println calls.
Example & Package examples/basic/src/main.cpp, library.json	Example: added baud and mode state variables, getters/setter and attribute wiring (baudAttr, modeAttr, option arrays) and registered them. library.json version bumped 0.1.2 → 0.1.3.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Protocol
    participant Attribute
    participant Transport
    participant Serial

    Client->>Protocol: request: cmdAttributes / describe
    Protocol->>Attribute: describe(out)
    Attribute->>Transport: out.write(string/int/bool)
    Transport->>Serial: print(...) / println(...)
    Serial-->>Transport: ok
    Transport-->>Attribute: written
    Attribute-->>Protocol: done
    Protocol-->>Client: response complete

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Possibly related PRs

• Fix wrong end of frame marker #7: Modifies the same output abstraction/transport layer and method naming (related changes to ISlvCtrlOut/transport implementations).
• Memory optimizations #2: Touches protocol I/O and attribute layers and example attribute wiring, overlapping with ListAttribute and example updates.

Poem

🐰 From send to write I nimbly hop,
options lined in a tidy crop,
baud and mode now take their place,
version bumped — a tiny race,
nibble the code and onward hop! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.13% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add missing list attribute' directly aligns with the main change in the PR, which introduces a new ListAttribute template class in SlvCtrlProtocol.h.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/add-missing-list-attr

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

src/SlvCtrlProtocol.h (2)

253-263: options_ stores a non-owning pointer — document the lifetime requirement.

The constructor decays the array reference to a const T* stored in options_. If a caller ever passes a stack-local array, the attribute will hold a dangling pointer. This is consistent with how name_ and attrs_ are handled elsewhere in the codebase, but unlike those (almost always string literals / file-scope arrays), an options array is slightly more tempting to construct locally.

Consider adding a brief comment on the constructor (or in class-level docs) noting that the array must outlive the ListAttribute instance.

---

305-319: Nit: containsStr_ returns a pointer, not a bool — consider renaming to findStr_.

containsInt_ returns bool while containsStr_ returns const char*. Naming the latter findStr_ would better express the return semantics and reduce cognitive friction when reading the setFromCString call site.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[heavyrubberslave]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/SlvCtrlProtocol.h`:
- Around line 289-302: Describe currently emits unquoted string options (in
describe/ writeOption_ for the string-specialization), which makes outputs like
rw[str(off|on|auto)] ambiguous when options contain '|', ')' or ']'; update the
logic so string options are safely quoted or escaped per the protocol: either
wrap option values in quotes (e.g., "value") or escape special chars before
writing; implement this in writeOption_ (or the code path used by describe for T
!= int32_t) so all entries in options_ are emitted quoted/escaped and include
tests to ensure describe produces rw[str("off"|"on"|"a|b")]-style output
matching the protocol rule.

🧹 Nitpick comments (2)

examples/basic/src/main.cpp (1)

38-42: Inconsistent declaration style: missing static and & operator.

baudAttr and modeAttr are missing the static keyword, unlike all other attribute declarations on lines 33–36. Similarly, function pointers are passed without the & prefix (e.g., getBaud vs. &getSpeed). Both forms are valid C++, but the inconsistency in example code may confuse users.

Proposed fix for consistency

-ListAttribute<int32_t> baudAttr("baud", getBaud, nullptr, kBaudOpts);
+static ListAttribute<int32_t> baudAttr("baud", &getBaud, nullptr, kBaudOpts);
 
 static const char* kModeOpts[] = { "off", "on", "auto" };
-ListAttribute<const char*> modeAttr("mode", getMode, setMode, kModeOpts);
+static ListAttribute<const char*> modeAttr("mode", &getMode, &setMode, kModeOpts);

src/SlvCtrlProtocol.h (1)

241-263: ListAttribute constructor: stores a pointer to a caller-owned array — document the lifetime requirement.

The constructor captures options as a raw const T*. The caller must ensure the array outlives the ListAttribute instance. In the example code, file-scope arrays satisfy this, but a stack-local array passed here would be a use-after-free. Consider adding a brief comment on the lifetime contract.

Suggested doc comment

 public:
+  // `options` must outlive this attribute (pointer is stored, not copied).
   template <size_t N>
   ListAttribute(const char* name,