Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new intermediate APDS9960 example that combines the RGBC sensor with the SSD1327 OLED to create a “reactive color lamp” UI that adapts background brightness to ambient light and shows a dominant-color label plus raw RGBC readings.
Changes:
- Added
color_lamp.pyexample that reads APDS9960 RGBC values and renders a round-screen-friendly UI on SSD1327. - Implemented a startup ambient-light calibration step to derive a dynamic clear-channel baseline for background brightness mapping.
- Added display shutdown behavior on exit via
display.power_off()in afinallyblock.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/apds9960/examples/color_lamp.py
Outdated
| # Add a small buffer to the max calibration so it doesn't max out too easily | ||
| MAX_CLEAR = max_clear_calib + (max_clear_calib // CALIB_BUFFER_DIVISOR) | ||
| print("Calibration complete. MAX_CLEAR set to:", MAX_CLEAR) |
There was a problem hiding this comment.
MAX_CLEAR is computed at runtime during calibration but is named like a module constant. Consider renaming to something like max_clear/max_clear_limit (and keeping true constants uppercase) to avoid implying it’s compile-time/static.
There was a problem hiding this comment.
Good catch regarding the PEP 8 naming conventions. You're absolutely right, since it's computed at runtime it shouldn't be formatted as a static constant. I've renamed it to max_clear_limit.
| except KeyboardInterrupt: | ||
| print("\nColor lamp stopped.") | ||
| finally: | ||
| # Clean up and power off display on exit | ||
| display.fill(0) | ||
| display.show() | ||
| sleep_ms(100) | ||
| display.power_off() |
There was a problem hiding this comment.
The example enables the APDS9960 light sensor but never disables it on exit. Since you’re already doing display cleanup in finally, consider calling apds.disable_light_sensor() (and/or apds.power_off()) there as well to avoid leaving the sensor powered and drawing current after Ctrl+C.
There was a problem hiding this comment.
Excellent point on hardware cleanup. I've added apds.disable_light_sensor() to the finally block to ensure the sensor doesn't keep drawing current after the script exits.
83892eb to
0c59829
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
💡 Suggestion: cette PR utilise le SSD1327 directement. Le projet a maintenant Tu pourrais utiliser : from steami_screen import Screen, SSD1327Display
display = SSD1327Display(ssd1327.WS_OLED_128X128_SPI(spi, dc, res, cs))
screen = Screen(display)
screen.clear()
screen.title("Color")
screen.value(color_name)
screen.bar(brightness, max_val=255)
screen.show()Le wrapper |
ReviewTrès bon travail sur cet exemple. L’ensemble est globalement propre, mais il y a quelques points à améliorer pour le rendre plus robuste et mieux aligné avec le projet. 1. Division potentiellement dangereusebg_color = (clamped_c * 15) // max_clear_limitSi Même si peu probable avec le baseline actuel, il vaut mieux sécuriser. Suggestion : if max_clear_limit <= 0:
max_clear_limit = 1
bg_color = (clamped_c * 15) // max_clear_limit2. Clamp incohérentclamped_c = max(0, min(c, max_clear_limit))Tu clamps avec Cela crée une incohérence entre :
À harmoniser pour avoir un comportement cohérent. 3. Détection de couleur trop sensibleif r > g and r > b:La détection est très sensible au bruit, ce qui peut provoquer du flickering entre couleurs. Suggestion : ajouter un seuil de dominance : THRESHOLD = 20
if r > g + THRESHOLD and r > b + THRESHOLD:4. Pas de normalisation RGBLes valeurs RGB sont utilisées directement : r = apds.red_light()
g = apds.green_light()
b = apds.blue_light()Le problème est qu’elles dépendent fortement de la lumière ambiante ( Suggestion : normaliser avec if c > 0:
r_n = r / c
g_n = g / c
b_n = b / cPuis utiliser ces valeurs pour la détection. 5. Calibration perfectible (non bloquant)max_clear_calib = max(max_clear_calib, c)Tu prends uniquement le maximum, ce qui est sensible à un pic ponctuel. Ce n’est pas bloquant, mais une moyenne serait plus robuste. 6. Utilisation de steami_screen (suggestion)Tu utilises directement Depuis, le projet a introduit Ce n’était pas encore disponible au moment de la PR, mais tu peux maintenant t’en servir pour simplifier le code. 7. README non mis à jourL’exemple est ajouté mais n’est pas référencé dans le README du driver. À faire :
8. Description de la PRIl manque la référence à l’issue dans la description. À ajouter : ConclusionTrès bon exemple, visuel et interactif, avec une vraie valeur pédagogique. À corriger principalement :
Avec ces ajustements, ce sera un excellent exemple pour le repository. |
0c59829 to
db8a050
Compare
|
Refactored color_lamp.py based on review feedback and hardware testing. Improvements : Decisions : |
Summary
closes #331
Adds an intermediate example (
color_lamp.py) demonstrating a reactive color lamp using the APDS9960 RGBC sensor and the SSD1327 OLED display. The screen dynamically changes its background brightness based on the room's ambient light and detects the dominant color of objects placed near it.Changes
lib/apds9960/examples/color_lamp.py.MAX_CLEARbaseline.display.power_off()) in thefinallyblock.Checklist
ruff checkpassespython -m pytest tests/ -k mock -vpasses (no mock test broken)<scope>: <Description.>format