Add functions to control the LED of an access point #261
Conversation
WalkthroughThe changes introduce three new methods to the Changes
Sequence DiagramsequenceDiagram
participant User
participant Controller
participant UniFi API
User->>Controller: setLED(device_id, color, brightness, mode)
Controller->>UniFi API: REST API Request
UniFi API-->>Controller: Update LED Settings
Controller-->>User: Confirmation
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
unifi.js (4)
2142-2162: Add input validation and improve documentationThe implementation looks good but could benefit from some improvements:
Add parameter validation:
- Validate device_id is a 24 char string
- Validate override_color is a valid hex color code
- Validate override_color_brightness is between 0-100
- Validate override_mode is one of: 'off', 'on', 'default'
Enhance JSDoc documentation with:
Here's a suggested implementation with the improvements:
/** * Override LED settings for a device (using REST) * + * @param {string} device_id - 24 char string; value of _id for the device which can be obtained from the device list + * @param {string} override_color - Hex color code e.g. "#ff0000" for red + * @param {number} override_color_brightness - Value between 0-100 for LED brightness + * @param {('off'|'on'|'default')} override_mode - LED mode: 'off' disables LED, 'on' enables LED, 'default' uses site setting + * @throws {Error} If parameters are invalid + * @returns {Promise<boolean>} True on success + * @example + * // Set device LED to bright red + * await controller.setLED('5f1234567890abcdef123456', '#ff0000', 100, 'on') */ setLED(device_id, override_color, override_color_brightness, override_mode) { + // Validate parameters + if (!/^[0-9a-f]{24}$/i.test(device_id)) { + throw new Error('Invalid device_id format'); + } + if (!/^#[0-9a-f]{6}$/i.test(override_color)) { + throw new Error('Invalid override_color format'); + } + if (!Number.isInteger(override_color_brightness) || + override_color_brightness < 0 || + override_color_brightness > 100) { + throw new Error('override_color_brightness must be between 0-100'); + } + if (!['off', 'on', 'default'].includes(override_mode)) { + throw new Error('override_mode must be one of: off, on, default'); + } return this._request( '/api/s/<SITE>/rest/device/' + device_id.trim(), { led_override: override_mode, led_override_color: override_color, led_override_color_brightness: override_color_brightness }, 'PUT' ); }
2164-2172: Add input validation and improve documentation for brightness controlSimilar improvements needed as the main setLED method:
- Add parameter validation
- Enhance JSDoc documentation
- Consider adding a brightness range constant
Here's a suggested implementation with the improvements:
+ /** @const {number} Maximum LED brightness value */ + const MAX_LED_BRIGHTNESS = 100; /** * Override LED Brightness for a device (using REST) * + * @param {string} device_id - 24 char string; value of _id for the device which can be obtained from the device list + * @param {number} override_color_brightness - Value between 0-100 for LED brightness + * @throws {Error} If parameters are invalid + * @returns {Promise<boolean>} True on success + * @example + * // Set device LED to 50% brightness + * await controller.setLEDBrightness('5f1234567890abcdef123456', 50) */ setLEDBrightness(device_id, override_color_brightness) { + // Validate parameters + if (!/^[0-9a-f]{24}$/i.test(device_id)) { + throw new Error('Invalid device_id format'); + } + if (!Number.isInteger(override_color_brightness) || + override_color_brightness < 0 || + override_color_brightness > MAX_LED_BRIGHTNESS) { + throw new Error(`override_color_brightness must be between 0-${MAX_LED_BRIGHTNESS}`); + } return this._request( '/api/s/<SITE>/rest/device/' + device_id.trim(), {led_override_color_brightness: override_color_brightness}, 'PUT' ); }
2174-2182: Add input validation and improve documentation for color controlSimilar improvements needed as the other LED control methods:
- Add parameter validation
- Enhance JSDoc documentation
- Consider adding color constants for common values
Here's a suggested implementation with the improvements:
+ /** @const {Object.<string, string>} Common LED colors */ + const LED_COLORS = { + RED: '#ff0000', + GREEN: '#00ff00', + BLUE: '#0000ff', + WHITE: '#ffffff', + OFF: '#000000' + }; /** * Override LED Color for a device (using REST) * + * @param {string} device_id - 24 char string; value of _id for the device which can be obtained from the device list + * @param {string} override_color - Hex color code e.g. "#ff0000" for red + * @throws {Error} If parameters are invalid + * @returns {Promise<boolean>} True on success + * @example + * // Set device LED to red + * await controller.setLEDColor('5f1234567890abcdef123456', LED_COLORS.RED) */ setLEDColor(device_id, override_color) { + // Validate parameters + if (!/^[0-9a-f]{24}$/i.test(device_id)) { + throw new Error('Invalid device_id format'); + } + if (!/^#[0-9a-f]{6}$/i.test(override_color)) { + throw new Error('Invalid override_color format'); + } return this._request( '/api/s/<SITE>/rest/device/' + device_id.trim(), {led_override_color: override_color}, 'PUT' ); }
2142-2182: Consider architectural improvements for LED controlWhile the implementation is solid, consider these architectural improvements:
Extract common validation logic into helper methods:
_validateDeviceId(device_id) _validateColor(color) _validateBrightness(brightness)Add error handling for API responses specific to LED control failures
Add unit tests to verify:
- Parameter validation
- API call formatting
- Error handling
- Success cases
Consider adding a higher-level LED control interface that combines these operations:
async configureLED(device_id, {color, brightness, mode}) { // Validate all parameters once // Make a single API call instead of multiple }Would you like me to help create a new GitHub issue to track these improvements?
|
Hello @jens-maus, a quick review followed by a new release (as you have resolved CVE-2024-39338 with the axios patch) would be highly appreciated 👍 Best regards and happy new year Thiemo |
Hello @jens-maus,
It's kinda funny to mostly see the same good people in Open-Source! Keep up the good work 👍
This PR adds some functionality to control the LED Brightness and color (e.g. for E7)
A npm release after this PR would be highly appreciated 😃
Best regards
Thiemo
Summary by CodeRabbit
setLED: Override LED settings with color, brightness, and mode.setLEDBrightness: Adjust the brightness of the LED.setLEDColor: Change the LED color using hex format.