Skip to content

Take advantage of the refactored ANIMartRIX to call the new _base met…#345

Open
netmindz wants to merge 1 commit intoMoonModules:mdevfrom
netmindz:ANIMartRIX-configurable
Open

Take advantage of the refactored ANIMartRIX to call the new _base met…#345
netmindz wants to merge 1 commit intoMoonModules:mdevfrom
netmindz:ANIMartRIX-configurable

Conversation

@netmindz
Copy link
Copy Markdown
Collaborator

@netmindz netmindz commented Mar 7, 2026

…hods with varying parameters at runtime

Summary by CodeRabbit

  • New Features
    • Enhanced and consolidated animation effects with improved customization: new controls for width/scale/size, speed, distance/zoom offsets, variant selection, and HSV mode options for richer visuals and finer tuning.
  • Chores
    • Updated Animartrix library dependency to the latest release.

@netmindz netmindz requested a review from softhack007 March 7, 2026 15:05
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 7, 2026

📝 Walkthrough

Walkthrough

The PR bumps the animartrix library commit in platformio.ini and consolidates multiple Animartrix usermod animation modes into parameterized variants, adding a local mapping helper and updating effect registrations in the usermod header.

Changes

Cohort / File(s) Summary
Library Dependency
platformio.ini
Updated animartrix_lib_deps commit hash from 81eb09b to 932337e.
Animartrix Usermod — Mode Consolidation & Registration
usermods/usermod_v2_animartrix/usermod_v2_animartrix.h
Replaced multiple distinct mode/data pairs with consolidated parameterized modes (e.g., Module_Experiment10→Module_Experiment810, SM9/10→SM910, SM5/6→SM56, Complex_Kaleido variants→Complex_Kaleido12, RGB_Blobs variants→RGB_Blobs45/23, Experiment2/3→Experiment23). Added animartrix_mapf(...) helper, updated parameter mappings from SEGMENT fields (custom/check), and adjusted effect registrations to bind new mode functions and _data_FX_mode_* constants. Net lines: +63 / -84.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I mapped the hues and numbers tight,

Six new hops from ten took flight,
A helper carrot paved the way,
Old modes condensed to dance and play,
🥕✨ — a rabbit cheers the code tonight

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.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 PR title is truncated and incomplete, cutting off mid-word ('_base met…'), making it impossible to fully understand the intended change despite referencing the refactored ANIMartRIX and _base methods mentioned in the PR objectives. Complete the PR title with the full objective: 'Take advantage of the refactored ANIMartRIX to call the new _base methods with varying parameters at runtime' to clearly convey the main intent of the changeset.
✅ 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 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

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

@softhack007
Copy link
Copy Markdown
Collaborator

@netmindz I need to think about how to get the new sliders aligned with extra sliders i've added in #344 🤔

image

Most of the visual and audio sliders from #344 could also go into segment settings. Otherwise we'd need more sliders for effects

  • 3 extra custom sliders (custom4 ... custom6)
  • 3 extra checkboxes (check4 ... check6)

@softhack007
Copy link
Copy Markdown
Collaborator

@coderabbitai please re-review this PR.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

@softhack007 Sure! I'll kick off a fresh review of this PR right away.


🧠 Learnings used
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR `#270`, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR `#270`, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue `#272`; do not address it within PR `#270`.
✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
usermods/usermod_v2_animartrix/usermod_v2_animartrix.h (1)

208-216: Consider: dir_idx derivation couples direction selection with dist_offset.

The current logic derives both dist_offset and dir_idx from custom1, meaning users cannot independently control the distance offset and direction. If this coupling is intentional for visual consistency between the merged effects, this is fine. Otherwise, consider using a separate checkbox (check1) for dir_idx similar to other combined effects.

// Current: dir_idx derived from custom1 threshold
int dir_idx = (SEGMENT.custom1 < 128) ? 3 : 0;

// Alternative: Use check1 for independent control
// bool use_dir3 = SEGMENT.check1;
// int dir_idx = use_dir3 ? 3 : 0;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@usermods/usermod_v2_animartrix/usermod_v2_animartrix.h` around lines 208 -
216, The dir_idx in mode_Module_Experiment23 is currently derived from
SEGMENT.custom1, coupling direction with dist_offset; change it to use an
independent control (e.g., SEGMENT.check1) so users can set direction
separately: read SEGMENT.check1 into a boolean (or similar) and set dir_idx
based on that instead of the custom1 threshold, then call
anim.Module_Experiment23_base(speed, dist_offset, dist_scale, dir_idx) as
before; update any UI/comment to reflect that dir_idx now comes from check1
rather than custom1.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@usermods/usermod_v2_animartrix/usermod_v2_animartrix.h`:
- Around line 208-216: The dir_idx in mode_Module_Experiment23 is currently
derived from SEGMENT.custom1, coupling direction with dist_offset; change it to
use an independent control (e.g., SEGMENT.check1) so users can set direction
separately: read SEGMENT.check1 into a boolean (or similar) and set dir_idx
based on that instead of the custom1 threshold, then call
anim.Module_Experiment23_base(speed, dist_offset, dist_scale, dir_idx) as
before; update any UI/comment to reflect that dir_idx now comes from check1
rather than custom1.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f867c3e4-96cb-4d7c-ac07-e0db699318fc

📥 Commits

Reviewing files that changed from the base of the PR and between 32bc474 and be5ce80.

📒 Files selected for processing (2)
  • platformio.ini
  • usermods/usermod_v2_animartrix/usermod_v2_animartrix.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants