Add Nodes

[ewowi]

Summary by CodeRabbit

• New Features

  • Added "Car Lights" layout for vehicle-style lighting.
  • Ring layouts now support rotation, scale, directional (clockwise) and partial-ring rendering.

• Bug Fixes

  • Fixed negative-coordinate handling in light positioning.

• Improvements

  • Read-only address fields render appropriate links (standard http for IPs, .local for mDNS names).
  • Layout naming/structure updated for clarity.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)

Validation error: Unrecognized key(s) in object: 'review', 'context'

⚙️ Configuration instructions

• Please see the configuration documentation for more information.
• You can also validate your configuration using the online YAML validator.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

📝 Walkthrough

Walkthrough

Refines read-only field rendering for IP vs mDNS, bumps a build driver version, clamps negative coordinates in PhysicalLayer::addLight, replaces RingsLayout with Rings241Layout and adds CarLightsLayout, and refactors RingLayout (topLeft → ringCenter plus rotation/scale/clockwise/doNextPin and partial-circle rendering).

Changes

Cohort / File(s)	Summary
Field rendering interface/src/lib/components/moonbase/FieldRenderer.svelte	Split read-only branch: property.type == "ip" renders standard http link; property.type == "mDNSName" renders http://{value}.local.
Build config platformio.ini	Bumped HP_ALL_VERSION build flag from "20251120" to "20260120".
Physical layer src/MoonLight/Layers/PhysicalLayer.cpp	Clamp negative position.x/y/z to 0 at start of addLight.
Module drivers src/MoonLight/Modules/ModuleDrivers.h	Replace RingsLayout with Rings241Layout in add/alloc paths; add CarLightsLayout exposure/allocation.
Layouts refactor & new layout src/MoonLight/Nodes/Layouts/L_MoonLight.h	Rename topLeft → ringCenter; add rotation, scale, clockwise, doNextPin; rewrite RingLayout onLayout to compute positions from ringCenter, support partial-circle inclusion and gating of nextPin; rename RingsLayout → Rings241Layout; update Rings16Layout placements; add new CarLightsLayout composing RingLayout instances.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• moving pin high and low out of the define, handling low level rs485  #91: Modifies ModuleDrivers.h layout exposure/allocation logic — strongly related to the Rings241/CarLights changes.
• rename UI components #75: Touches interface/src/lib/components/moonbase/FieldRenderer.svelte — related to the field-rendering adjustments.

Poem

🐰 Around a centre, LEDs spin and glow,
Rings reshaped, new lights in tow,
Coordinates clamped, links now true,
Drivers updated, versions too,
I hop and hum — a bright hello! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Add Nodes' is vague and does not convey meaningful information about the changeset; it fails to specify which nodes are being added or what the actual changes entail.	Revise the title to be more specific about the primary change, such as 'Refactor RingLayout and add CarLightsLayout' or 'Add CarLightsLayout and refactor coordinate handling in ring layouts'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing touches

• 📝 Generate docstrings

---

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

[ewowi]

@coderabbitai, can you review commit 322ce85

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the commit details and diff
gh api repos/MoonModules/MoonLight/commits/322ce8518a0529fd8b898086efaeb96038250786 --jq '{message: .commit.message, author: .commit.author.name, date: .commit.author.date, files: [.files[] | {filename: .filename, status: .status, additions: .additions, deletions: .deletions, changes: .changes}]}'

# Get the full diff for the commit
gh api repos/MoonModules/MoonLight/commits/322ce8518a0529fd8b898086efaeb96038250786 --jq '.files[] | "=== \(.filename) ===\n\(.patch // "Binary or no patch available")\n"'

Length of output: 13596

---

@ewowi, I've reviewed commit 322ce85. Here's my analysis:

✅ Good Changes

FieldRenderer.svelte

• Properly separates IP and mDNS name link handling
• mDNS names now correctly append .local suffix

platformio.ini

• Version bump from "20251120" to "20260120" (tracking driver updates)

PhysicalLayer.cpp

• Defensive coordinate clamping prevents negative values from causing issues

⚠️ Issues Found

1. Loss of Z-Coordinate in RingLayout (Line 522 in L_MoonLight.h)

addLight({(int)x, (int)y, 0});

The z-coordinate from ringCentre.z is hardcoded to 0. Should be:

addLight({(int)x, (int)y, ringCentre.z});

2. Semantic Change Impact in Rings16Layout

The refactoring changes topLeft (which was an offset/corner) to ringCentre (actual center position). However, in Rings16Layout (Lines 551-583), you're adding a constant offset d = 5 to all positions:

ringLayout.ringCentre = {cols[5] + d, rows[3] + d, 0};

This constant offset suggests the coordinate system semantics changed, but it's unclear if this correctly preserves the original ring positions. Consider:

• Were the original rows[] and cols[] arrays defining top-left corners?
• Do they now need different adjustments per ring based on their radius?

3. Float Precision in Rotation Logic (Lines 490-518)

Direct float comparisons like rotation == 0 and rotation >= 360 may have precision issues. Consider using epsilon-based comparisons:

if (rotation < 1.0f || rotation >= 360.0f) {
  includeLED = true;
}

4. Silent Coordinate Clamping (PhysicalLayer.cpp, Lines 202-204)

Negative coordinates are silently clamped to 0 without logging. This could hide layout calculation bugs. Consider adding a debug log when clamping occurs:

if (position.x < 0) { 
  EXT_LOGD(ML_TAG, "Clamping negative x: %d", position.x);
  position.x = 0; 
}

5. CarLightsLayout State Management

In CarLightsLayout::onLayout(), you reuse a single RingLayout instance and modify properties like clockwise and rotation between calls. While this works, ensure state is explicitly set for each configuration to avoid subtle bugs if the class evolves.

💡 Suggestions

1. Angle calculation consistency: You changed from dividing by 359 to 360 (Line 457) - this is mathematically correct ✅

2. Scale application: The scale is applied to radius and center coordinates, which is good, but verify the CarLightsLayout hardcoded positions work correctly at different scales.

3. Documentation: Consider adding comments explaining the doNextPin flag in RingLayout, as it's a non-obvious control for composition scenarios.

Summary

The refactoring is substantial and well-structured, but please address the z-coordinate loss issue (#1) and verify the Rings16Layout positioning (#2) works correctly. The other issues are optional improvements for robustness.

Would you like me to help verify the coordinate transformations or test any specific aspect of these changes?