Update Examples to use Widgets #260
Conversation
- Replace ItemList with ItemWidget - Update string arrays to const char* - Modify menu items to use WidgetList
WalkthroughThe changes in this pull request primarily affect the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used📓 Learnings (1)src/ItemList.h (1)🔇 Additional comments (4)src/ItemList.h (2)
The deprecation message effectively communicates the transition to widgets while maintaining brevity.
The src/ItemRangeBase.h (2)
The
Fix the line length formatting issue. The deprecation attribute line still exceeds the maximum line length and needs to be wrapped according to clang-format rules. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
- Added ItemWidget.h and WidgetList.h - Replaced ITEM_STRING_LIST with ITEM_WIDGET for colors
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range comments (1)
examples/List/List.ino (1)
Widget implementation is consistent, but String usage needs attention
The widget system implementation in
List.inois correct and follows the new pattern. However, there are two remaining instances whereStringarrays are used instead ofconst char*:
examples/SimpleRotary/SimpleRotary.ino: UsingString colors[]examples/ButtonWithRotaryAdapter/ButtonWithRotaryAdapter.ino: UsingString colors[]These should be migrated to
const char*arrays for consistency with the new implementation.🔗 Analysis chain
Line range hint
1-43: Verify complete migration to widget system.Let's ensure the migration to widgets is complete and consistent across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining callback implementations echo "Checking for old callback pattern..." rg -l "Callback.*List" examples/ # Verify widget adoption in other examples echo "Checking widget adoption in other examples..." rg -l "ITEM_WIDGET|WIDGET_LIST" examples/ # Look for potential String to const char* migration opportunities echo "Checking for remaining String usage..." rg -l "extern String" examples/Length of output: 406
Script:
#!/bin/bash # Let's do a more comprehensive check for widget patterns and old implementations # Check for any old list-related patterns echo "Checking for old list patterns..." rg -g '!*.md' "class.*List" examples/ -A 5 # Look for widget usage patterns echo "Checking widget usage patterns..." rg -g '!*.md' "WIDGET_" examples/ -A 2 # Check for callback patterns in headers echo "Checking callback patterns in headers..." rg -g '*.h' "callback" include/ -A 2 # Look for menu item patterns echo "Checking menu item patterns..." rg -g '!*.md' "ITEM_" examples/ -A 2 # Check for any String usage in the codebase echo "Checking String usage..." rg -g '!*.md' "String " examples/Length of output: 10931
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- examples/List/List.ino (1 hunks)
🔇 Additional comments (3)
examples/List/List.ino (3)
1-7: LGTM: Widget header inclusion aligns with PR objective.The addition of
<widget/WidgetList.h>properly supports the transition to widget-based functionality.
12-13: Great improvement: More efficient string handling.The transition from
extern Stringtoconst char*brings several benefits:
- More efficient memory usage
- Compile-time memory allocation
- Removes String object overhead
- Eliminates external linkage dependency
Line range hint
36-43: LGTM: Setup and loop maintain clean separation of concerns.The unchanged initialization and event loop demonstrate good compatibility with the new widget system.
- Added ItemWidget.h and WidgetList.h - Replaced ITEM_STRING_LIST with ITEM_WIDGET for colors
- Define constants for the count of colors and numbers in each list. - Modify code to use these constants for array sizes instead of hardcoding values.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- examples/ButtonWithRotaryAdapter/ButtonWithRotaryAdapter.ino (1 hunks)
- examples/List/List.ino (1 hunks)
- examples/SimpleRotary/SimpleRotary.ino (1 hunks)
🔇 Additional comments (7)
examples/SimpleRotary/SimpleRotary.ino (3)
4-4: LGTM: New widget-related includes are properly organized.The addition of ItemWidget.h and WidgetList.h headers aligns with the transition to a widget-based approach.
Also applies to: 11-11
20-21: Great optimization: Using const char instead of String.*The change from String to const char* reduces memory usage, as it avoids dynamic memory allocation. The COLORS_COUNT calculation is also a robust way to maintain the array size.
26-29: Verify consistent widget implementation across examples.The widget-based approach looks good, but let's ensure consistency across all example files.
✅ Verification successful
Let me check the actual implementation in those files to verify consistency.
Widget implementation is consistent across examples
The verification shows that:
- No String[] usage was found in the examples
- ITEM_WIDGET with WIDGET_LIST is consistently implemented across all three examples
- The widget pattern for handling const char* color lists is identical in all files
- Each example properly uses the same format specifier "%s" and callback pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining String usage in examples echo "Checking for remaining String usage in examples:" rg -l "String\[\]" examples/ # Check for consistent widget implementation echo "Checking widget implementation in other examples:" rg -l "ITEM_STRING_LIST|ITEM_WIDGET" examples/Length of output: 418
Script:
#!/bin/bash # Check the actual widget implementations echo "SimpleRotary implementation:" rg "ITEM_" examples/SimpleRotary/SimpleRotary.ino -B 2 -A 2 echo -e "\nList implementation:" rg "ITEM_" examples/List/List.ino -B 2 -A 2 echo -e "\nButtonWithRotaryAdapter implementation:" rg "ITEM_" examples/ButtonWithRotaryAdapter/ButtonWithRotaryAdapter.ino -B 2 -A 2Length of output: 1659
examples/ButtonWithRotaryAdapter/ButtonWithRotaryAdapter.ino (2)
7-7: LGTM: Header changes align with widget-based approach.The addition of widget-related headers properly supports the transition to the new widget-based implementation.
Also applies to: 15-15
30-33: 🧹 Nitpick (assertive)LGTM: Clean widget implementation with inline callback.
The widget-based approach with an inline lambda function improves code readability by keeping related logic together.
Let's verify the Serial usage pattern in other examples:
Consider adding Serial connection verification in the lambda for robustness:
ITEM_WIDGET( "Color", - [](const char* color) { Serial.println(color); }, + [](const char* color) { + if (Serial) Serial.println(color); + }, WIDGET_LIST(colors, COLORS_COUNT, 0, "%s")),examples/List/List.ino (2)
12-13: Good job switching fromStringtoconst char*arraysUsing
const char*overStringobjects helps prevent memory fragmentation and reduces dynamic memory allocation, which is good practice in embedded programming.
15-16: Good job replacing magic numbers with calculated constantsUsing
sizeof()to calculateNUMS_COUNTandCOLORS_COUNTimproves maintainability and reduces errors if the arrays change size.
- Refactored color arrays in multiple files to use the static keyword for consistency. - Updated the count variables for colors and numbers arrays.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- examples/ButtonWithRotaryAdapter/ButtonWithRotaryAdapter.ino (1 hunks)
- examples/List/List.ino (1 hunks)
- examples/SimpleRotary/SimpleRotary.ino (1 hunks)
🔇 Additional comments (9)
examples/List/List.ino (3)
1-11: LGTM! Widget include is properly added.The include statements are well-organized, and the new widget include supports the transition to the widget-based approach.
15-16: LGTM! Array size constants are well implemented.Using sizeof to calculate array sizes is a good practice that avoids magic numbers and automatically updates when arrays change.
Note: The previous review comment about using PROGMEM for string arrays is still valid.
22-29: Verify widget functionality with various inputs.The widget implementation looks clean and uses the new constants correctly. However, let's verify the widget behavior.
✅ Verification successful
Let me gather more information about the widget implementation to ensure consistency.
Let me check one more aspect to verify the widget implementation completely.
Let me check the widget implementation in these header files to ensure everything is properly defined.
Widget implementation is correctly used and functioning as expected
The implementation is verified to be working correctly based on the following findings:
- The
WIDGET_LISTtemplate is properly implemented with type safety and error handling- The widget implementation is consistent across all example files
- The color list widget is properly initialized with correct array size calculation using
COLORS_COUNT- The lambda callbacks are properly typed and will work with the
ItemWidgettemplate implementation- The widget's value changes are properly handled through the
processmethod with proper bounds checking🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify widget implementation consistency across examples # Test: Check if other example files use similar widget patterns rg -A 2 "ITEM_WIDGET.*Color" examples/ rg -A 2 "WIDGET_LIST.*colors" examples/Length of output: 812
Script:
#!/bin/bash # Check the definition of COLORS_COUNT and colors array across examples rg -A 5 "COLORS_COUNT" examples/ rg -A 5 "const char\* (const )?colors\[\]" examples/ # Check the ITEM_WIDGET macro definition and usage rg -A 5 "^#define ITEM_WIDGET" --type cpp rg -A 5 "ITEM_WIDGET.*\[.*\].*WIDGET" examples/Length of output: 4545
Script:
#!/bin/bash # Check the WIDGET_LIST macro definition and related widget implementations rg -A 3 "^#define WIDGET_LIST" --type cpp rg -A 3 "^#define ITEM_WIDGET" --type cpp # Look for any widget-related header files fd -e h -e hpp | xargs rg "WIDGET|widget" -lLength of output: 328
Script:
#!/bin/bash # Check widget list implementation cat src/widget/WidgetList.h # Check item widget implementation cat src/ItemWidget.h # Check base widget implementations for any relevant constraints cat src/widget/BaseWidget.hLength of output: 8548
examples/SimpleRotary/SimpleRotary.ino (3)
4-4: LGTM: New includes are appropriate.The addition of ItemWidget.h and WidgetList.h headers properly supports the new widget-based implementation.
Also applies to: 11-11
20-21: Great optimization: Memory-efficient string handling.The changes improve memory efficiency in two ways:
- Using
static const char*instead ofStringreduces RAM usage- The
COLORS_COUNTcalculation is a clean way to maintain array size automatically
26-29: LGTM: Clean widget implementation.The widget-based approach for color selection is well-structured and provides a good example of using the WIDGET_LIST feature.
examples/ButtonWithRotaryAdapter/ButtonWithRotaryAdapter.ino (3)
7-7: LGTM: Header includes align with widget migration.The addition of widget-related headers properly supports the transition to a widget-based approach.
Also applies to: 15-15
24-25: Previous feedback implemented correctly.
30-33: 🧹 Nitpick (assertive)Consider these improvements to the widget implementation:
- Verify if Serial output is intended for production
- The lambda parameter could be more descriptively named
- The format string "%s" is redundant for simple string output
ITEM_WIDGET( "Color", - [](const char* color) { Serial.println(color); }, + [](const char* selectedColor) { Serial.println(selectedColor); }, WIDGET_LIST(colors, COLORS_COUNT, 0, "%s")),Let's verify the Serial usage pattern:
- Removed ItemIntRange and added WidgetRange for the "Dist" item to improve functionality and consistency in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- examples/SimpleRotary/SimpleRotary.ino (1 hunks)
🔇 Additional comments (3)
examples/SimpleRotary/SimpleRotary.ino (3)
3-3: LGTM: New includes support widget functionality.The added headers properly support the transition to widget-based implementations.
Also applies to: 10-11
20-21: LGTM: Memory-efficient string storage.Good optimization using
static const char*instead ofStringobjects, which reduces memory usage and heap fragmentation. TheCOLORS_COUNTcalculation is also a safe approach.
26-29: LGTM: Clean widget implementations.Both widgets are well-implemented:
- Color selector uses appropriate list widget with proper formatting
- Distance selector has good range configuration with unit display
Also applies to: 31-34
- Update item references to use new widget structure - Rename test from "ItemRangeBase Test" to "WidgetRange Test"
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- examples/IntFloatValues/IntFloatValues.ino (2 hunks)
- examples/SimpleRotary/SimpleRotary.ino (1 hunks)
- test/IntFloatValues.test.yml (1 hunks)
🧰 Additional context used
📓 Learnings (1)
examples/SimpleRotary/SimpleRotary.ino (1)
Learnt from: forntoh PR: forntoh/LcdMenu#260 File: examples/SimpleRotary/SimpleRotary.ino:24-35 Timestamp: 2024-10-29T00:48:28.782Z Learning: In the `examples/SimpleRotary/SimpleRotary.ino` file, the user prefers not to add explanatory comments regarding widgets vs. traditional menu items, differences between `WIDGET_LIST` and `WIDGET_RANGE`, and format string usage.
🔇 Additional comments (8)
examples/IntFloatValues/IntFloatValues.ino (2)
1-1: LGTM! Headers properly updated for widget supportThe new includes for
ItemWidget.handwidget/WidgetRange.hare correctly added to support the transition to widgets.Also applies to: 7-7
Line range hint
1-27: Verify consistent widget implementation across examplesThe changes look good, but let's verify that the widget implementation is consistent across all example files.
✅ Verification successful
Let me gather more information about the widget implementations to ensure full consistency.
Widget implementation is consistent across examples
The verification shows consistent patterns across all example files:
- All examples properly include required widget headers (
ItemWidget.hand specific widget types)ITEM_WIDGETmacro usage follows the same structure: label, callback, and widget configuration- Widget configurations (
WIDGET_RANGE,WIDGET_LIST) are used with appropriate type-specific parameters- Callback declarations follow consistent naming and typing conventions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent widget usage patterns across examples # Test 1: Verify all examples use the new widget headers echo "Checking for widget header includes..." rg -l "ItemWidget.h|WidgetRange.h" "examples/" # Test 2: Verify consistent WIDGET_RANGE usage pattern echo "Checking WIDGET_RANGE usage patterns..." rg "WIDGET_RANGE\(" "examples/"Length of output: 711
Script:
#!/bin/bash # Check the full context of widget usage in each example echo "=== Checking widget-related includes across examples ===" rg -A 3 "^#include.*Widget" examples/ echo -e "\n=== Checking ITEM_WIDGET usage patterns ===" rg -B 2 -A 2 "ITEM_WIDGET\(" examples/ echo -e "\n=== Checking callback declarations ===" rg "callback.*\(" examples/Length of output: 5303
test/IntFloatValues.test.yml (3)
1-5: LGTM: Test metadata is properly updatedThe test name has been correctly updated to reflect the new widget-based approach, maintaining consistency with the codebase changes.
25-39: Verify the float range test sequenceThe test sequence for the float widget ("Curr") appears logically sound, testing:
- Edit mode entry
- Increment (-0.99 → -0.96)
- Decrement (-0.96 → -0.98)
- Edit mode exit
Let's verify if these test values align with the widget's implementation:
#!/bin/bash # Search for the float widget range definition to verify test values rg -A 5 "Curr.*ITEM_WIDGET" examples/
9-21: Verify the integer range test sequenceThe test sequence for the integer widget ("Dist") appears logically sound, testing:
- Edit mode entry
- Increment (101 → 103)
- Decrement (103 → 101)
- Edit mode exit
Let's verify if these test values align with the widget's implementation:
examples/SimpleRotary/SimpleRotary.ino (3)
3-3: LGTM! Widget-related includes are properly organized.The new includes are correctly placed and necessary for the widget functionality.
Also applies to: 10-11
20-21: LGTM! Memory-efficient implementation of colors array.The use of
static const char*instead ofStringis more memory efficient, and theCOLORS_COUNTcalculation is safe and correct.
26-29: LGTM! Clean widget implementations for both selectors.Both widgets are well-implemented:
- Color selector uses WIDGET_LIST with appropriate string formatting
- Distance selector uses WIDGET_RANGE with proper bounds, step size, and unit display
Also applies to: 31-34
|
Memory usage change @ e129860
Click for full report table
Click for full report CSV |
- Deprecated ItemList and ItemRangeBase classes for removal in future versions. Suggested using ITEM_WIDGET with WIDGET_LIST and WIDGET_RANGE respectively.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range comments (1)
src/ItemList.h (1)
Line range hint
143-143: Deprecate the ITEM_STRING_LIST macro.For consistency with the class deprecation, the macro should also be marked as deprecated.
-#define ITEM_STRING_LIST(...) (new ItemList(__VA_ARGS__)) +[[deprecated("ITEM_STRING_LIST is deprecated and will be removed in future versions. Please use ITEM_WIDGET with WIDGET_LIST instead.")]] +#define ITEM_STRING_LIST(...) (new ItemList(__VA_ARGS__))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
- src/ItemList.h (1 hunks)
- src/ItemRangeBase.h (1 hunks)
- test/IntFloatValues.test.yml (1 hunks)
- test/List.test.yml (1 hunks)
🔇 Additional comments (5)
test/List.test.yml (2)
9-9: LGTM: Color selection test cases updated correctly.The log message expectations for the color selection workflow have been consistently updated to use the new widget-based terminology (
ItemWidget::enterEditMode,WidgetList::nextValue,ItemWidget::exitEditMode), aligning with the transition fromItemListto Widgets.Also applies to: 11-11, 13-13, 15-15, 17-17
21-21: LGTM: Number selection test cases updated correctly.The log message expectations for the number selection workflow have been systematically updated to use the new widget-based terminology, maintaining consistency with the first section and the overall widget-based approach.
Also applies to: 23-23, 25-25, 27-27, 29-29, 31-31, 33-33
test/IntFloatValues.test.yml (3)
1-4: LGTM! Test name updated to reflect the widget-based approach.The renaming from "ItemRangeBase Test" to "WidgetRange Test" aligns well with the PR's objective of updating examples to use widgets.
9-21: LGTM! Widget test sequence for "Dist" parameter is well structured.The test steps properly verify:
- Edit mode entry/exit
- Increment/decrement functionality
- Serial output validation
25-39: LGTM! Widget test sequence handles floating-point values correctly.The test appropriately verifies widget functionality with floating-point values, maintaining consistent precision (-0.99 to -0.96) and proper state management.
- Changed the data type of numbers array from char* to uint8_t. - Updated the callback function parameter type to uint8_t for displaying numbers correctly.
|
Memory usage change @ ed267c2
Click for full report table
Click for full report CSV |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range comments (2)
test/List.test.yml (1)
Line range hint 1-33: Consider enhancing test organization with descriptive sections.
While the test coverage is comprehensive, consider structuring the test file with clear sections using YAML comments to improve maintainability:
name: List Test
version: 1
author: Thomas Forntoh
steps:
+ # Navigation to menu items
- wait-serial: "#LOG# LcdMenu::setScreen"
- simulate: downButton-press
- wait-serial: "#LOG# MenuScreen::down=1"
+ # Color widget test sequence
- simulate: enterButton-press
- wait-serial: "#LOG# ItemWidget::enterEditMode=Color"
[...]
+ # Number widget test sequence
- simulate: enterButton-press
- wait-serial: "#LOG# ItemWidget::enterEditMode=Num"
[...]src/ItemList.h (1)
Line range hint 147-147: Consider deprecating the ITEM_STRING_LIST macro.
Since ItemList is being deprecated, the macro that creates instances of it should also be marked as deprecated to guide users towards the new widget-based approach.
-#define ITEM_STRING_LIST(...) (new ItemList(__VA_ARGS__))
+#define ITEM_STRING_LIST(...) [[deprecated("Use ITEM_WIDGET with WIDGET_LIST instead")]] (new ItemList(__VA_ARGS__))🧰 Tools
🪛 GitHub Check: clang_format
[failure] 32-32: src/ItemList.h#L32
code should be clang-formatted [-Wclang-format-violations]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
- examples/List/List.ino (1 hunks)
- src/ItemList.h (1 hunks)
- src/ItemRangeBase.h (1 hunks)
- test/List.test.yml (1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/ItemList.h (1)
Learnt from: forntoh
PR: forntoh/LcdMenu#260
File: src/ItemList.h:15-15
Timestamp: 2024-10-29T01:30:35.961Z
Learning: In `src/ItemList.h`, the user prefers not to include code examples in deprecation messages, and wants to keep them concise.
🪛 GitHub Check: clang_format
src/ItemList.h
[failure] 32-32: src/ItemList.h#L32
code should be clang-formatted [-Wclang-format-violations]
src/ItemRangeBase.h
[failure] 47-47: src/ItemRangeBase.h#L47
code should be clang-formatted [-Wclang-format-violations]
🔇 Additional comments (7)
test/List.test.yml (3)
9-9: LGTM: Color widget test sequence is well structured.
The test sequence for color selection properly validates:
- Widget edit mode entry
- Sequential color navigation
- Edit mode exit
Also applies to: 11-11, 13-13, 15-15, 17-17
21-21: LGTM: Number widget test sequence is comprehensive.
The test sequence for number selection effectively covers:
- Widget edit mode entry
- Bidirectional navigation (both increment and decrement)
- Value boundaries testing
- Edit mode exit
Also applies to: 23-23, 25-25, 27-27, 29-29, 31-31, 33-33
9-33: Verify widget implementation matches test expectations.
The test file assumes specific log message formats from the widget implementation. Let's verify these assumptions against the actual implementation.
✅ Verification successful
The widget implementation matches the test expectations
The verification confirms that:
ItemWidgetlogsenterEditModeandexitEditModewith the expected format#LOG# ItemWidget::enterEditMode=<value>inBaseItemManyWidgets.hWidgetListlogsnextValueandpreviousValuewith the expected format#LOG# WidgetList::nextValue=<value>inwidget/WidgetList.h
The logging formats and behavior in the implementation exactly match what's being tested in test/List.test.yml.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the widget implementation produces the expected log messages
# Check for the widget class definitions and logging statements
echo "Checking widget implementation..."
rg -A 2 "class (ItemWidget|WidgetList)" --type cpp
# Verify logging format in the implementation
echo "Checking logging format..."
rg "#LOG#.*Widget" --type cpp
Length of output: 917
Script:
#!/bin/bash
# Let's check for the specific log messages mentioned in the test
echo "Checking for enterEditMode/exitEditMode logging..."
rg "enterEditMode|exitEditMode" --type cpp -A 2
echo "Checking for nextValue/previousValue logging..."
rg "nextValue|previousValue" --type cpp -A 2
# Let's also check the implementation files for these classes
echo "Checking implementation files..."
fd -e cpp -e h "WidgetList|ItemWidget" --exec cat {}
Length of output: 8844
examples/List/List.ino (3)
1-7: LGTM! Required headers are properly included.
The new widget-related headers are correctly included to support the widget-based implementation.
12-16: LGTM! Memory-efficient array declarations with proper size constants.
The changes improve memory efficiency by:
- Using
const char*instead ofString - Adding size constants to avoid magic numbers
22-29: Verify the WIDGET_LIST macro's boolean parameter usage.
The implementation looks clean, but let's verify the purpose of the last boolean parameter (true) in the WIDGET_LIST macro calls.
✅ Verification successful
The cycle parameter in WIDGET_LIST is correctly used
The cycle parameter (set to true) enables cycling through the list values. When enabled:
- When navigating past the last item, it wraps back to the first item
- When navigating before the first item, it wraps to the last item
This is appropriate for both widgets in the review:
- The colors list where cycling through color options is intuitive
- The numbers list where continuous navigation is desired
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for WIDGET_LIST macro documentation or other usages
rg -A 5 "WIDGET_LIST.*true" --type cpp
Length of output: 40
Script:
#!/bin/bash
# Search for WIDGET_LIST macro definition and documentation
rg -A 10 "WIDGET_LIST" --type cpp
Length of output: 1299
Script:
#!/bin/bash
# Search for WIDGET_LIST macro in header files
rg -A 10 "WIDGET_LIST" --type hpp --type h
Length of output: 70
Script:
#!/bin/bash
# Look for any files containing WIDGET_LIST in their name
fd "WIDGET_LIST"
Length of output: 16
Script:
#!/bin/bash
# Search for widget-related macros and their documentation
rg "#define.*WIDGET" --type cpp --type hpp --type h
Length of output: 81
Script:
#!/bin/bash
# Search for WidgetList class implementation to understand the cycle parameter
rg -B 5 -A 15 "class WidgetList" --type h
Length of output: 1305
Script:
#!/bin/bash
# Look for any documentation or comments about the cycle parameter
rg -B 2 -A 2 "cycle" --type h
Length of output: 4912
Script:
#!/bin/bash
# Search for other usages of WidgetList to understand the cycle parameter in context
rg "new WidgetList" --type cpp --type h -B 2 -A 2
Length of output: 548
src/ItemRangeBase.h (1)
Line range hint 47-56: Verify the deprecation notice impact.
The deprecation notice is clear and aligns with the broader transition to widget-based implementations across the codebase.
Let's verify the usage of this deprecated class to ensure all examples have been updated:
✅ Verification successful
Deprecation notice is properly implemented with successful migration path
The verification shows:
- Examples are correctly using the new
WIDGET_RANGEapproach withITEM_WIDGET - No direct usage of deprecated
ItemRangeBasein examples ItemFloatRangeandItemIntRangestill inherit fromItemRangeBasebut are also marked as deprecated through inheritance- Clear migration path exists as demonstrated in examples (SimpleRotary.ino and IntFloatValues.ino)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining uses of ItemRangeBase that need migration
# Test 1: Search for direct usage of ItemRangeBase
echo "Checking for ItemRangeBase usage..."
rg "ItemRangeBase" -l
# Test 2: Search for new widget usage to confirm migration
echo "Verifying migration to WIDGET_RANGE..."
rg "WIDGET_RANGE" -l
Length of output: 410
Script:
#!/bin/bash
# Let's check the content of examples to verify if they use the new WIDGET_RANGE approach
echo "Checking examples content for WIDGET_RANGE usage..."
rg -A 5 "WIDGET_RANGE" examples/
echo -e "\nChecking for any remaining ItemRangeBase usage in examples..."
rg "ItemRangeBase|ItemFloatRange|ItemIntRange" examples/
echo -e "\nChecking content of ItemFloatRange.h and ItemIntRange.h..."
cat src/ItemFloatRange.h src/ItemIntRange.h
Length of output: 3538
🧰 Tools
🪛 GitHub Check: clang_format
[failure] 47-47: src/ItemRangeBase.h#L47
code should be clang-formatted [-Wclang-format-violations]
Prevent clang-format from formatting specific sections of code by adding the `// clang-format off` directive above those sections.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
ItemListandItemRangeBaseclasses as deprecated, advising users to transition to the new widget system.