Add segment name

[mik-laj]

Proposed Changes

Expose n (segment name) field from WLED API in Segment model.

No behavioral changes, simple passthrough.

Related Issues

(Github link to related issues or pull requests)

Summary by CodeRabbit

Release Notes

• New Features

  • Segments can now be assigned custom names for better organization and identification.

• Tests

  • Updated test fixtures to validate segment naming functionality.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

These changes introduce a new optional name field to the Segment dataclass in the WLED models, configured with alias "n" for serialization compatibility. Test fixtures and spacing are updated accordingly to align with the new field definition.

Changes

Cohort / File(s)	Summary
Segment Model Enhancement src/wled/models.py	Adds optional name: str | None field to Segment dataclass with alias="n" for JSON serialization/deserialization.
Test & Fixture Updates tests/test_models.py, tests/fixtures/rgb.json	Updates fixture data to include the new segment n field with value "Ceiling" for segment id: 0, and adds spacing between test sections.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A segment now whispers its name with pride,
With alias "n" carried as its guide,
In fixtures and models, the change takes flight,
Naming segments clearly—what a delight! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add segment name' accurately reflects the main change: adding a new optional name field to the Segment dataclass, exposing the 'n' field from the WLED API.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/wled/models.py (2)

298-302: Tighten docstring and remove trailing whitespace

Clarify fallback wording, reference None correctly, and avoid trailing spaces to keep linters happy.

-    """The name of the segment. 
-    
-    Names are not present by default. If this is none, use 
-    "Segment{segment_id}" to match the WLED UI.
-    """
+    """User-defined segment name (alias: "n").
+
+    Not present by default. If None or empty, callers may fall back to a UI-like
+    default, e.g., "Segment {segment_id}".
+    """

---

297-303: Optional: expose a computed fallback to reduce duplication downstream

Consider providing a convenience property (and optional normalization) so consumers don’t have to replicate fallback logic.

Example (outside the changed hunk):

# Inside class Segment
@property
def effective_name(self) -> str:
    if self.name:
        return self.name
    if self.segment_id is not None:
        return f"Segment {self.segment_id}"
    return "Segment"

@classmethod
def __post_deserialize__(cls, obj: "Segment") -> "Segment":
    # Normalize empty string to None for consistency
    if obj.name == "":
        obj.name = None
    return obj

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0bdb185 and 6ed2188.

📒 Files selected for processing (1)

• src/wled/models.py (1 hunks)

🔇 Additional comments (1)

src/wled/models.py (1)

297-303: Addition of name (alias "n") looks correct

Matches WLED’s segment JSON field and will omit on serialization when None, which is desirable. Nice, minimal change.

[mik-laj]

@coderabbitai I pushed a fix, please review it.

[mik-laj]

I checked that this change is correct at the real device.

At the beginning, I didn't have any segments name

(wled-py3.12) vscode ➜ /workspaces/python-wled (segment-name) $ python test_segments.py
Device:
Segment 0:
  Name: None
  Effective name: Segment 0
Segment 1:
  Name: None
  Effective name: Segment 1

Then I set a name for two segments:

(wled-py3.12) vscode ➜ /workspaces/python-wled (segment-name) $ python test_segments.py
Device:
Segment 0:
  Name: Left window
  Effective name: Left window
Segment 1:
  Name: Right window
  Effective name: Right window

And later I deleted the name of the segment:

(wled-py3.12) vscode ➜ /workspaces/python-wled (segment-name) $ python test_segments.py
Device:
Segment 0:
  Name: Left window
  Effective name: Left window
Segment 1:
  Name: None
  Effective name: Segment 1

[github-actions]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[mik-laj]

This is waiting for review from the project maintainer.

[github-actions]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[mik-laj]

It is not stale. I am waiting for the project maintainer.

[github-actions]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[mik-laj]

Not stale

[github-actions]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[mik-laj]

Not stale.

[github-actions]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[mik-laj]

Not stale.

[Copilot]

Pull request overview

Adds support in the WLED Python client models to deserialize the user-defined segment name returned by WLED’s JSON API ("n"), enabling downstream consumers (e.g., Home Assistant) to display segment names instead of ID-based generated labels.

Changes:

• Add optional Segment.name field with Mashumaro alias mapping to API key "n".

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_models.py (1)

10-13: Make fixture decoding explicit for cross-platform determinism.

read_text() uses implicit encoding. Prefer explicit UTF-8 when loading JSON fixtures.

Proposed patch

 def load_fixture(file_name: str) -> dict:
     """Load a fixture file from the fixtures directory."""
     fixture_path = FIXTURE_DIR / file_name
-    return json.loads(fixture_path.read_text())
+    return json.loads(fixture_path.read_text(encoding="utf-8"))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_models.py` around lines 10 - 13, The load_fixture helper uses
implicit encoding via fixture_path.read_text(), which can yield
platform-dependent behavior; change load_fixture (referencing function name
load_fixture and symbol FIXTURE_DIR and fixture_path) to read the file with an
explicit UTF-8 decoding (e.g., call read_text with encoding="utf-8" or open the
file with encoding="utf-8") before passing the string into json.loads so JSON
decoding is deterministic across platforms.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/test_models.py`:
- Around line 10-13: The load_fixture helper uses implicit encoding via
fixture_path.read_text(), which can yield platform-dependent behavior; change
load_fixture (referencing function name load_fixture and symbol FIXTURE_DIR and
fixture_path) to read the file with an explicit UTF-8 decoding (e.g., call
read_text with encoding="utf-8" or open the file with encoding="utf-8") before
passing the string into json.loads so JSON decoding is deterministic across
platforms.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f16cfb7 and 3576706.

📒 Files selected for processing (3)

• tests/fixtures/state_with_segment_name.json
• tests/fixtures/state_without_segment_name.json
• tests/test_models.py

✅ Files skipped from review due to trivial changes (1)

• tests/fixtures/state_with_segment_name.json

[github-actions]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.17%. Comparing base (3e87d76) to head (764e9ef).
⚠️ Report is 577 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1810       +/-   ##
===========================================
+ Coverage   58.61%   98.17%   +39.56%     
===========================================
  Files           6        8        +2     
  Lines         662      878      +216     
  Branches      143       95       -48     
===========================================
+ Hits          388      862      +474     
+ Misses        270       13      -257     
+ Partials        4        3        -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[frenck]

Thanks, @mik-laj 👍

../Frenck

                       

Blogging my personal ramblings at frenck.dev