-
-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Output a clear "No Logitech Litra devices found" message from litra devices
when no devices are connected
#95
Conversation
…devices` when no devices are connected
WalkthroughThe changes in this pull request modify the Changes
Poem
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 42d3c2e in 6 seconds
More details
- Looked at
71
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. src/main.rs:296
- Draft comment:
Consider usingeprintln!
instead ofprintln!
for error messages to differentiate them from regular output. - Reason this comment was not posted:
Confidence changes required:50%
The PR adds a check for emptylitra_devices
and prints a message if no devices are found. This is a good addition for user feedback.
Workflow ID: wflow_MTVFtusdFTi9JsPw
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/main.rs (1)
295-327
: LGTM with minor suggestionsThe changes successfully address the PR objective by adding a clear message when no Logitech Litra devices are found. The implementation is correct and improves the user experience. Here are a couple of minor suggestions for improvement:
- The indentation of the
else
block (lines 297-325) is inconsistent with theif
statement. Consider adjusting the indentation for better readability.- The empty line at 327 is unnecessary and can be removed for consistency.
Here's a suggested diff to address these minor issues:
if litra_devices.is_empty() { println!("No Logitech Litra devices found"); } else { - for device_info in &litra_devices { - println!( - "- {} ({}): {} {}", - device_info.device_type, - device_info.serial_number, - get_is_on_text(device_info.is_on), - get_is_on_emoji(device_info.is_on) - ); + for device_info in &litra_devices { + println!( + "- {} ({}): {} {}", + device_info.device_type, + device_info.serial_number, + get_is_on_text(device_info.is_on), + get_is_on_emoji(device_info.is_on) + ); - println!(" - Brightness: {} lm", device_info.brightness_in_lumen); - println!( - " - Minimum: {} lm", - device_info.minimum_brightness_in_lumen - ); - println!( - " - Maximum: {} lm", - device_info.maximum_brightness_in_lumen - ); - println!(" - Temperature: {} K", device_info.temperature_in_kelvin); - println!( - " - Minimum: {} K", - device_info.minimum_temperature_in_kelvin - ); - println!( - " - Maximum: {} K", - device_info.maximum_temperature_in_kelvin - ); - } + println!(" - Brightness: {} lm", device_info.brightness_in_lumen); + println!( + " - Minimum: {} lm", + device_info.minimum_brightness_in_lumen + ); + println!( + " - Maximum: {} lm", + device_info.maximum_brightness_in_lumen + ); + println!(" - Temperature: {} K", device_info.temperature_in_kelvin); + println!( + " - Minimum: {} K", + device_info.minimum_temperature_in_kelvin + ); + println!( + " - Maximum: {} K", + device_info.maximum_temperature_in_kelvin + ); + } } - Ok(())
Important
Add message in
handle_devices_command()
to indicate no Logitech Litra devices are found.handle_devices_command()
inmain.rs
, added a check for emptylitra_devices
and print "No Logitech Litra devices found" if true.This description was created by for 42d3c2e. It will automatically update as commits are pushed.
Summary by CodeRabbit