Conversation
There was a problem hiding this comment.
Pull request overview
Expands the lib/apds9960 driver README to provide feature coverage, basic usage, and an API reference comparable to other drivers in this repository.
Changes:
- Add new README sections for Features, I2C address, Basic Usage, API grouped by feature, configuration snippets, exceptions, and examples.
- Update the mpremote example invocation to use the repo’s
lib/<driver>mounting style. - Add notes about usage patterns (lazy activation, gesture polling) and available driver classes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Parfait — voici une version **alignée sur le style ism330dl / wsen-hids**, avec wording homogène, phrases courtes et impératives, et structure cohérente. | ||
|
|
There was a problem hiding this comment.
Remove the French editorial sentence ("Parfait — …") from the README. It reads like an internal drafting note and breaks the otherwise user-facing English documentation style.
| Parfait — voici une version **alignée sur le style ism330dl / wsen-hids**, avec wording homogène, phrases courtes et impératives, et structure cohérente. |
| ## API | ||
|
|
There was a problem hiding this comment.
There are two "## API" headings back-to-back (one at the start of the API section and another before "Ambient Light & Color"). This duplicates the top-level section and makes the outline confusing; keep a single "## API" header and nest the feature sections under it.
| ## API |
|
|
||
| * `get_prox_int_high_thresh()` / `set_prox_int_high_thresh(value)` | ||
| Get or set high proximity threshold | ||
|
|
There was a problem hiding this comment.
The API list under Proximity documents get_prox_int_low_thresh() / get_prox_int_high_thresh(), but the driver also exposes get_proximity_int_low_threshold() / set_proximity_int_low_threshold() and get_proximity_int_high_threshold() / set_proximity_int_high_threshold() as public methods. Since this README claims a complete public API, these should be documented as well (or the README should clarify which set is preferred).
| * `get_prox_int_high_thresh()` / `set_prox_int_high_thresh(value)` | |
| Get or set high proximity threshold | |
| * `get_proximity_int_low_threshold()` / `set_proximity_int_low_threshold(value)` | |
| Get or set low proximity threshold (alternative method names) | |
| * `get_prox_int_high_thresh()` / `set_prox_int_high_thresh(value)` | |
| Get or set high proximity threshold | |
| * `get_proximity_int_high_threshold()` / `set_proximity_int_high_threshold(value)` | |
| Get or set high proximity threshold (alternative method names) |
| * The driver automatically enables sensors when needed (lazy activation). | ||
| * Gesture detection requires continuous polling via `gesture()`. |
There was a problem hiding this comment.
The note about "automatically enables sensors when needed (lazy activation)" is a bit inaccurate/ambiguous: light/proximity reads do lazy-enable internally, but gesture() does not (it returns APDS9960_DIR_NONE unless gesture mode is enabled). Please tighten this wording to avoid suggesting that gesture is also auto-enabled.
| * The driver automatically enables sensors when needed (lazy activation). | |
| * Gesture detection requires continuous polling via `gesture()`. | |
| * Ambient light and proximity reads automatically enable their respective sensors when needed (lazy activation). | |
| * Gesture detection is not auto-enabled; enable gesture mode first (via the corresponding enable method) before continuously polling via `gesture()`. |
| * `APDS9960InvalidDevId` — invalid device ID detected | ||
| * `APDS9960InvalidMode` — invalid mode selection |
There was a problem hiding this comment.
There are a couple of trailing spaces in this section (e.g., after "selection"), which can cause unintended Markdown line breaks in some renderers. Please trim trailing whitespace.
|
README réécrit en f225ec4 pour traiter tous les commentaires : Corrections :
Copilot :
Prête à merger. |
|
🎉 This PR is included in version 0.0.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Closes #189
Parent issue #194
Description
Expand the
apds9960README to match the structure and level of detail of other drivers (e.g.ism330dl,wsen-hids).Changes
Notes