Skip to content

feat(steami_config): Add magnetometer calibration storage.#241

Merged
nedseb merged 7 commits intomainfrom
feat/steami-config-mag-cal
Mar 25, 2026
Merged

feat(steami_config): Add magnetometer calibration storage.#241
nedseb merged 7 commits intomainfrom
feat/steami-config-mag-cal

Conversation

@nedseb
Copy link
Copy Markdown
Contributor

@nedseb nedseb commented Mar 25, 2026

Summary

Add helpers to persist and restore hard-iron and soft-iron calibration offsets for the LIS2MDL magnetometer.

New API

config = SteamiConfig(flash)
config.load()

# Store calibration after running calibrate_minmax_3d()
config.set_magnetometer_calibration(
    hard_iron_x=mag.x_off, hard_iron_y=mag.y_off, hard_iron_z=mag.z_off,
    soft_iron_x=mag.x_scale, soft_iron_y=mag.y_scale, soft_iron_z=mag.z_scale,
)
config.save()

# Restore calibration at boot
config.apply_magnetometer_calibration(mag)

Key decisions

  • JSON key cm (compact) with hx/hy/hz/sx/sy/sz sub-keys
  • Missing keys default to 0.0 (offsets) or 1.0 (scales) for backward compatibility
  • apply_magnetometer_calibration() guards on class name LIS2MDL (like temperature calibration)
  • Example uses SPI OLED screen with countdown, MENU button to start

Files changed

  • lib/steami_config/steami_config/device.py — 3 new methods
  • lib/steami_config/README.md — Magnetometer calibration API documented
  • lib/steami_config/examples/calibrate_magnetometer.py — Full calibration example with OLED
  • tests/scenarios/steami_config.yaml — 5 mock tests + 3 hardware tests

Tests

Mock (5)

  • Set and get magnetometer calibration
  • Get returns None when not set
  • Apply calibration to LIS2MDL instance (all 6 fields)
  • Apply does nothing when not set (all 6 fields verified)
  • Calibration survives save/load round-trip (all 6 fields)

Hardware (3)

  • Save/load round-trip on real flash
  • Apply to real LIS2MDL sensor
  • Coexistence with temperature calibration

Closes #170

Copilot AI review requested due to automatic review settings March 25, 2026 11:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds persistent storage and restore/apply helpers for LIS2MDL magnetometer hard-iron (offset) and soft-iron (scale) calibration within SteamiConfig, along with scenario-based mock tests to validate behavior and persistence.

Changes:

  • Add set_magnetometer_calibration(), get_magnetometer_calibration(), and apply_magnetometer_calibration() to SteamiConfig, persisted under a compact JSON key.
  • Extend the steami_config scenario suite with mock tests covering set/get/apply and save/load round-trip.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
lib/steami_config/steami_config/device.py Introduces magnetometer calibration persistence and an apply helper for LIS2MDL instances.
tests/scenarios/steami_config.yaml Adds mock scenarios validating magnetometer calibration set/get/apply and persistence across save/load.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/scenarios/steami_config.yaml Outdated
dev2 = SteamiConfig(dev._flash)
dev2.load()
cal = dev2.get_magnetometer_calibration()
result = cal["hard_iron_x"] == 5.5 and cal["soft_iron_x"] == 1.0
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The save/load round-trip scenario only checks hard_iron_x and soft_iron_x, so it won’t catch issues with the other four values (or defaulting behavior for missing soft-iron values). Consider asserting all six values after reload to better cover persistence correctness.

Suggested change
result = cal["hard_iron_x"] == 5.5 and cal["soft_iron_x"] == 1.0
result = (
cal["hard_iron_x"] == 5.5
and cal["hard_iron_y"] == -2.2
and cal["hard_iron_z"] == 0.3
and cal["soft_iron_x"] == 1.0
and cal["soft_iron_y"] == 1.0
and cal["soft_iron_z"] == 1.0
)

Copilot uses AI. Check for mistakes.
Comment on lines +176 to +186
cm = self._data.get("cm")
if cm is None:
return None
return {
"hard_iron_x": cm["hx"],
"hard_iron_y": cm["hy"],
"hard_iron_z": cm["hz"],
"soft_iron_x": cm["sx"],
"soft_iron_y": cm["sy"],
"soft_iron_z": cm["sz"],
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_magnetometer_calibration() will raise KeyError if the stored cm dict is present but missing any of the expected keys (hx/hy/hz/sx/sy/sz). Since config data can come from older firmware versions or be partially corrupted while still being valid JSON, consider treating missing/invalid entries as “not set” (return None) or applying defaults for the missing fields, instead of crashing.

Copilot uses AI. Check for mistakes.
Comment on lines +188 to +205
def apply_magnetometer_calibration(self, lis2mdl_instance):
"""Apply stored magnetometer calibration to a LIS2MDL instance.

The instance must have x_off/y_off/z_off and x_scale/y_scale/z_scale
attributes.

Args:
lis2mdl_instance: a LIS2MDL driver instance.
"""
cal = self.get_magnetometer_calibration()
if cal is None:
return
lis2mdl_instance.x_off = cal["hard_iron_x"]
lis2mdl_instance.y_off = cal["hard_iron_y"]
lis2mdl_instance.z_off = cal["hard_iron_z"]
lis2mdl_instance.x_scale = cal["soft_iron_x"]
lis2mdl_instance.y_scale = cal["soft_iron_y"]
lis2mdl_instance.z_scale = cal["soft_iron_z"]
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apply_magnetometer_calibration() currently applies calibration to any object passed in (it will silently add x_off/x_scale attributes if they don’t already exist). For consistency with apply_temperature_calibration() (which checks the sensor class name), consider guarding on type(lis2mdl_instance).__name__.lower() == "lis2mdl" (or similar) and returning otherwise, to avoid accidentally mutating unrelated objects.

Copilot uses AI. Check for mistakes.
Comment on lines +161 to +168
self._data["cm"] = {
"hx": hard_iron_x,
"hy": hard_iron_y,
"hz": hard_iron_z,
"sx": soft_iron_x,
"sy": soft_iron_y,
"sz": soft_iron_z,
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linked issue (#170) describes a different persisted JSON schema (cal_mag with hix/hiy/...) than the implementation here (cm with hx/hy/...). If any external docs/tools were written against the issue’s schema, this will be confusing; consider aligning the on-flash keys to the agreed schema or explicitly documenting the final schema (and any migration/back-compat) in a repo doc/CHANGELOG entry.

Copilot uses AI. Check for mistakes.
Comment thread tests/scenarios/steami_config.yaml Outdated

mag = LIS2MDL()
dev.apply_magnetometer_calibration(mag)
result = mag.x_off == 0 and mag.x_scale == 1
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The “does nothing when not set” scenario only asserts x_off and x_scale, so a regression that mutates y_off/z_off/y_scale/z_scale would still pass. Consider asserting all six calibration fields remain unchanged to make this test fully cover the no-op behavior.

Suggested change
result = mag.x_off == 0 and mag.x_scale == 1
result = (
mag.x_off == 0
and mag.y_off == 0
and mag.z_off == 0
and mag.x_scale == 1
and mag.y_scale == 1
and mag.z_scale == 1
)

Copilot uses AI. Check for mistakes.
@nedseb
Copy link
Copy Markdown
Contributor Author

nedseb commented Mar 25, 2026

All 5 review comments addressed in d711f3c:

  1. Round-trip test — Now asserts all 6 values after reload.
  2. KeyError on partial dataget_magnetometer_calibration() uses .get() with defaults (0.0 for offsets, 1.0 for scales).
  3. Type guardapply_magnetometer_calibration() now checks type().__name__.lower() == "lis2mdl" before applying.
  4. Schema mismatch — Issue steami_config: Add magnetometer calibration storage. #170 had a preliminary schema. The final schema (cm with hx/hy/hz/sx/sy/sz) is documented in README.md. Will update the issue.
  5. No-op test — Now asserts all 6 fields remain unchanged.

@nedseb nedseb merged commit 017a40b into main Mar 25, 2026
4 checks passed
@nedseb nedseb deleted the feat/steami-config-mag-cal branch March 25, 2026 13:37
semantic-release-updater Bot pushed a commit that referenced this pull request Mar 25, 2026
# [0.1.0](v0.0.3...v0.1.0) (2026-03-25)

### Features

* **steami_config:** Add magnetometer calibration storage. ([#241](#241)) ([017a40b](017a40b))
@semantic-release-updater
Copy link
Copy Markdown

🎉 This PR is included in version 0.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

steami_config: Add magnetometer calibration storage.

2 participants