steami_config: Add accelerometer calibration storage.#323
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Charly-sketch
changed the title
There was a problem hiding this comment.
Pull request overview
Adds persistent accelerometer bias calibration support for the STeaMi config zone and integrates per-axis accelerometer offset correction into the ISM330DL driver, with accompanying examples, docs, and scenario tests (mock + hardware).
Changes:
- Add
SteamiConfighelpers to store/load/apply accelerometer bias offsets under JSON keycal_accel. - Add accelerometer offset support to
ISM330DLand apply offsets inacceleration_g()(propagating toms2andorientation()). - Add scenario tests plus a calibration example and documentation updates.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/scenarios/steami_config.yaml | Adds mock + hardware scenario coverage for accelerometer calibration persistence and application. |
| tests/scenarios/ism330dl.yaml | Adds mock + hardware scenario coverage for accel offset getters/setters and offset effects on readings/orientation. |
| lib/steami_config/steami_config/device.py | Implements set/get/apply_accelerometer_calibration() storing cal_accel in config JSON. |
| lib/steami_config/examples/calibrate_accelerometer.py | New example to compute, persist, reload, and verify accelerometer bias offsets. |
| lib/steami_config/README.md | Documents accelerometer calibration API and JSON schema update. |
| lib/ism330dl/ism330dl/device.py | Adds _accel_offset_* state and set/get_accel_offset(), applying offsets in acceleration_g(). |
| lib/ism330dl/README.md | Documents accelerometer offset correction and how to apply persistent calibration via steami_config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nedseb
left a comment
nedseb
left a comment
There was a problem hiding this comment.
La PR est bien structurée : driver, config, exemple, tests mock + hardware, et READMEs mis à jour. Quelques retours ci-dessous.
✅ Points positifs
- Pattern cohérent avec la calibration magnétomètre existante (
set_*,get_*,apply_*) — bonne approche - Tests complets : mock + hardware + coexistence avec les autres calibrations (le test "coexists with temperature calibration" est excellent)
- Exemple bien pensé : mesure → calcul offset → sauvegarde → reload → vérification
- Propagation correcte : les offsets dans
acceleration_g()se propagent àacceleration_ms2()etorientation()
📝 À corriger
1. README steami_config : code Python cassé dans le bloc (ligne 132)
Copilot l'a déjà signalé : dans la section "Store calibration", il y a du texte brut (Read calibration) à l'intérieur du bloc de code Python. Ça rend le copier-coller invalide. Il faut soit séparer en deux blocs, soit commenter :
### Store calibration
\`\`\`python
config.set_accelerometer_calibration(ox=0.01, oy=-0.02, oz=0.03)
\`\`\`
### Read calibration
\`\`\`python
cal = config.get_accelerometer_calibration()
# -> {"ox": 0.01, "oy": -0.02, "oz": 0.03} or None
\`\`\`2. Ligne vide en trop dans device.py (steami_config)
Il y a une double ligne vide après set_accelerometer_calibration() (après la fermeture du dict self._data["cal_accel"]). Une seule suffit entre les méthodes — c'est la convention du reste du fichier.
3. Clé JSON : cal_accel vs convention existante
La calibration magnétomètre utilise "cm" (clé courte) et les offsets sont "hx", "hy", "hz", "sx", "sy", "sz". La calibration température utilise "tc" avec "g" et "o".
La nouvelle calibration utilise "cal_accel" avec "ox", "oy", "oz" — c'est plus long et inconsistant avec le pattern existant de clés courtes. Le JSON doit tenir dans 1 KB (config zone). Je suggère :
# Au lieu de "cal_accel" → "ca" (calibration accelerometer)
# Cohérent avec "cm" (calibration magnetometer) et "tc" (temperature calibration)
self._data["ca"] = {"ox": float(ox), "oy": float(oy), "oz": float(oz)}Ce n'est pas bloquant mais c'est important pour la cohérence et la taille JSON. À discuter avec Sébastien.
4. Vérification de type par string dans apply_accelerometer_calibration
if type(ism330dl_instance).__name__.lower() != "ism330dl":
returnC'est le même pattern que apply_magnetometer_calibration — donc c'est cohérent. Mais il faut noter que c'est fragile : si quelqu'un sous-classe ISM330DL, la vérification échouera silencieusement. Un hasattr(ism330dl_instance, 'set_accel_offset') serait plus robuste (duck typing). Ce n'est pas à corriger ici car c'est un pattern existant — mais à garder en tête pour une future harmonisation.
5. L'exemple calibrate_accelerometer.py suppose az ≈ -1g (screen up)
Ligne 51 : oz = az + 1.0
C'est correct pour la STeaMi posée écran vers le haut (Z pointe vers le bas, donc az ≈ -1g, et l'offset attendu est az - (-1) = az + 1). Mais un commentaire expliquant le signe serait utile :
# Board flat, screen up: expected (0, 0, -1g)
# Offset = measured - expected
ox = ax - 0.0
oy = ay - 0.0
oz = az - (-1.0) # az + 1.06. Manque un --- entre les sections du README ism330dl
Le bloc "Accelerometer calibration" est ajouté mais il manque un séparateur --- entre "Get offsets" et "Notes on persistent calibration" pour rester cohérent avec le reste du README.
🔧 Résumé des actions
| # | Action | Priorité |
|---|---|---|
| 1 | Fix README steami_config code block | Bloquant |
| 2 | Supprimer double ligne vide dans device.py | Mineur |
| 3 | Discuter cal_accel → ca |
À discuter |
| 4 | (Rien à faire — note pour le futur) | — |
| 5 | Ajouter commentaire sur le signe dans l'exemple | Recommandé |
| 6 | Ajouter --- manquant dans README ism330dl |
Mineur |
Bonne PR dans l'ensemble, le plus important est le fix du README (#1). Le reste est cosmétique.
in calibrate accel example
|
|
🎉 This PR is included in version 0.12.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary
Add support for persistent accelerometer bias calibration (XYZ) for the ISM330DL.
This PR introduces helpers to store, retrieve, and apply accelerometer offsets using the config zone.
Closes #171
Part of #166
Depends on #167
Changes
Add accelerometer calibration storage in
steami_configset_accelerometer_calibration(ox, oy, oz)get_accelerometer_calibration()apply_accelerometer_calibration(sensor)Store calibration in config zone using JSON key
cal_accelAdd accelerometer offset support in
ISM330DL_accel_offset_x/y/zattributesset_accel_offset()andget_accel_offset()acceleration_g()(propagates toms2andorientation)Add example:
calibrate_accelerometer.pyAdd mock tests for:
Add hardware tests for:
Update README:
steami_config: new "Accelerometer Calibration" section + JSON formatism330dl: accelerometer offset support documentationChecklist
ruff checkpassespython -m pytest tests/ -k mock -vpasses (no mock test broken)<scope>: <Description.>formatTest Results
'steami_config' tests
'ism330dl' tests