Skip to content

Conversation

@blazoncek
Copy link
Collaborator

@blazoncek blazoncek commented Nov 2, 2025

When changing segment's sliders, differs(), in json.cpp, will return changed state. However it will not do the same if checkmarks are changed.
This PR solves the issue.

Summary by CodeRabbit

  • Refactor
    • Enhanced internal change detection mechanisms for improved consistency.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 2, 2025

Walkthrough

Adds three new field comparisons to the differs function in json.cpp that trigger SEG_DIFFERS_FX when check1, check2, or check3 fields differ between segment objects. No behavioral or control-flow changes introduced.

Changes

Cohort / File(s) Summary
Field Comparison Logic
wled00/json.cpp
Adds three new field difference checks (check1, check2, check3) to the differs function that set SEG_DIFFERS_FX flag when these fields differ

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single file modification with repetitive pattern (three identical comparison additions)
  • Straightforward logic additions with no control-flow complexity
  • Primary focus: verify that the three new field comparisons are correctly implemented and that SEG_DIFFERS_FX is the appropriate flag to set

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Add segment checkmarks to differs() check" accurately and directly reflects the main change in the changeset. The modification adds three new field comparisons (check1, check2, check3) to the differs() function to detect changes in segment checkmarks, which is precisely what the title conveys. The title is concise, uses clear and specific terminology, and avoids vague language or noise; a teammate scanning the repository history would immediately understand that this PR adds checkmark detection to the differs function.
✨ 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 add-check-diff

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b60313e and 1da2692.

📒 Files selected for processing (1)
  • wled00/json.cpp (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.cpp

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use 2-space indentation for C++ source files (.cpp)

Files:

  • wled00/json.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: wled_build / Build Environments (esp32c3dev)
  • GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
  • GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
  • GitHub Check: wled_build / Build Environments (esp32_wrover)
  • GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
  • GitHub Check: wled_build / Build Environments (esp32_eth)
  • GitHub Check: wled_build / Build Environments (lolin_s2_mini)
  • GitHub Check: wled_build / Build Environments (usermods)
  • GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
  • GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
  • GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
  • GitHub Check: wled_build / Build Environments (esp32dev)
  • GitHub Check: wled_build / Build Environments (esp8266_2m_160)
  • GitHub Check: Analyze (c-cpp)
🔇 Additional comments (1)
wled00/json.cpp (1)

54-56: LGTM! Checkmark detection correctly implemented.

The three new comparisons follow the exact same pattern as the slider checks (custom1-3) above them, using the appropriate SEG_DIFFERS_FX flag. The implementation is consistent with how these fields are defined in the SegmentCopy struct (lines 34-36), initialized (lines 108-110), deserialized (lines 302-304), and serialized (lines 627-629). This completes the differs() function by ensuring all segment parameters trigger change detection, resolving the issue described in the PR objectives.


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.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants