Skip to content

fix(steami_config): Simplify calibrate_temperature for frozen drivers.#249

Merged
nedseb merged 3 commits intomainfrom
fix/simplify-frozen-imports
Mar 26, 2026
Merged

fix(steami_config): Simplify calibrate_temperature for frozen drivers.#249
nedseb merged 3 commits intomainfrom
fix/simplify-frozen-imports

Conversation

@nedseb
Copy link
Copy Markdown
Contributor

@nedseb nedseb commented Mar 26, 2026

Summary

Simplify calibrate_temperature.py example by removing RAM management hacks that were needed before drivers were frozen into the firmware.

Closes #248

Changes

Removed:

  • sys.modules.pop() / __import__() pattern for loading/unloading drivers one at a time
  • gc.collect() calls between sensor reads
  • Dynamic module import via string names

Replaced with:

  • Direct from xxx import XXX imports (drivers are in ROM, not RAM)
  • Simple list of (name, sensor_instance) tuples
  • Added docstring note about requiring frozen firmware (make firmware && make deploy)

Related

Test plan

  • make lint passes
  • make run SCRIPT=lib/steami_config/examples/calibrate_temperature.py on hardware

Copilot AI review requested due to automatic review settings March 26, 2026 07:46
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

This PR simplifies the steami_config temperature calibration example now that sensor drivers can be frozen into firmware, removing previous RAM-management workarounds.

Changes:

  • Replaced dynamic imports / sys.modules.pop() / gc.collect() with direct imports.
  • Refactored calibration to iterate over a static list of sensors.
  • Updated the module docstring to note the requirement for frozen-firmware workflow (make firmware && make deploy).

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

Comment on lines +34 to 40
# Calibrate each sensor
sensors = [
("hts221", HTS221(i2c)),
("wsen_pads", WSEN_PADS(i2c)),
("lis2mdl", LIS2MDL(i2c)),
("ism330dl", ISM330DL(i2c)),
]
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

sensors is built with already-constructed driver instances. This means if any sensor constructor raises (device missing / wrong address / I2C error), the script aborts before calibrating any sensors, and it also keeps all sensor instances alive at once. Consider storing (name, DriverClass) (or a small factory) and instantiating each sensor inside the loop (and del it after) so one failure doesn’t prevent calibrating the others and to keep RAM use bounded.

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

Choose a reason for hiding this comment

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

Acknowledged. With frozen drivers, all instances share the same ROM code so RAM impact is minimal. We'll keep the current approach (all instantiated upfront) and revisit if hardware tests show RAM issues (#175).

sensor = cls(i2c)
if config_name == "ism330dl":
sleep_ms(200)
for name, sensor in sensors:
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

The “Verification (after reload)” loop reuses the same sensor instances created before config.save()/reload. This doesn’t match the real next-boot scenario and can hide issues that only show up on fresh instances (driver init state, cached calibration fields, etc.). Recreate new sensor instances for verification after config2.load() (and apply the same ISM330DL warm-up delay if it’s needed on first read).

Suggested change
for name, sensor in sensors:
verification_sensors = [
("hts221", HTS221(i2c)),
("wsen_pads", WSEN_PADS(i2c)),
("lis2mdl", LIS2MDL(i2c)),
("ism330dl", ISM330DL(i2c)),
]
for name, sensor in verification_sensors:
if name == "ism330dl":
sleep_ms(200)

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

Choose a reason for hiding this comment

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

Good point. Fixed in 1836fb4 — verification now creates fresh sensor instances to simulate a real reboot.

@nedseb nedseb merged commit 81f9588 into main Mar 26, 2026
3 checks passed
@nedseb nedseb deleted the fix/simplify-frozen-imports branch March 26, 2026 08:37
semantic-release-updater Bot pushed a commit that referenced this pull request Mar 26, 2026
## [0.1.2](v0.1.1...v0.1.2) (2026-03-26)

### Bug Fixes

* **steami_config:** Simplify calibrate_temperature for frozen drivers. ([#249](#249)) ([81f9588](81f9588))
@semantic-release-updater
Copy link
Copy Markdown

🎉 This PR is included in version 0.1.2 🎉

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.

fix(steami_config): Simplify examples for frozen driver imports.

2 participants