Restructure IMU devices from Enum class to dict, populated during import using entry points

[CatYang3]

Summary

Describe the main changes in this PR:

• Added a registry that registers all IMU devices in [project.entry-points."imu_module.devices"] and adds them to a dict called IMU_DEVICES.
• The old Enum class IMUDevices no longer exists. The methods that match the addresses to devices are now standalone.
• This structure allows projects downstream to add their own IMUs or modify the settings of existing IMUs without forking/modifying this module, by adding to their pyproject.toml.
• The existing configs are now in the builtin_devices.py file to avoid circular import.

---

Code Quality Checklist

Before requesting review, ensure:

• All CI/CD pipelines pass
• Code includes docstrings and type hints
• I added new tests where necessary
• I ran make tree to update the README.md

---

Additional Notes

• Anything else reviewers should know?
• Attach outputs, plots, logs, or GIFs here.

[NGierden]

@claude-review

[claude]

Claude finished @NGierden's task in 2m 8s —— View job

---

Code Review

• Read changed files
• Review registry.py and entry point mechanism
• Review builtin_devices.py
• Review devices.py changes
• Review factory.py changes
• Review tests
• Review pyproject.toml changes
• Compile and post full review

---

Summary

The approach is solid — replacing the IMUDevices enum with an entry-point-based registry is the right design for extensibility, and the imu_module.device_overrides group for downstream config patching is a nice touch. There are two bugs to fix before merging, plus a few test issues.

---

🐛 Bug: Key typo breaks LSM6DSOX_LIS3MDL override lookup

src/imu_python/registry.py, line 21

# Current (wrong)
registry["LIS6DSOX_LIS3MDL"] = LSM6DSOX_LIS3MDL

# Should be
registry["LSM6DSOX_LIS3MDL"] = LSM6DSOX_LIS3MDL

The sensor is LSM6DSOX (L-S-M), not LIS6DSOX. The wrong key means any downstream project trying to override LSM6DSOX_LIS3MDL via imu_module.device_overrides will silently insert a duplicate entry instead of replacing the original. The _from_address scan still works (it iterates all values), but the override mechanism is broken for this sensor.

Fix this →

---

🐛 Bug: factory.py bypasses the registry for the I2C scan fallback

src/imu_python/factory.py, lines 8 and 119

from imu_python.builtin_devices import MOCK  # direct import, not from registry
...
logger.warning(f"I2C scan failed: {err}. Returning {MOCK} addresses.")
return [a for d in MOCK.devices.values() for a in d.addresses]

If a downstream project overrides MOCK via imu_module.device_overrides, the fallback addresses in scan_i2c_bus still use the original hardcoded config. Should use the registry:

from imu_python.definitions import MOCK_NAME
from imu_python.registry import IMU_DEVICES
...
mock = IMU_DEVICES[MOCK_NAME]
logger.warning(f"I2C scan failed: {err}. Returning {mock} addresses.")
return [a for d in mock.devices.values() for a in d.addresses]

Fix this →

---

⚠️ No error handling on entry point load failures

src/imu_python/registry.py, lines 23–26

for ep in entry_points(group="imu_module.devices"):
    config: IMUConfig = ep.load()   # no validation, no error handling
    registry[ep.name] = config

If a downstream package misconfigures their entry point (wrong type, import error, missing dependency), this will crash the entire module import for all users. Two things would help:

for ep in entry_points(group="imu_module.devices"):
    try:
        config = ep.load()
        if not isinstance(config, IMUConfig):
            logger.warning(f"Entry point {ep.name!r} did not return an IMUConfig, skipping")
            continue
        registry[ep.name] = config
        logger.info(f"loaded IMU config {ep.name!r}")
    except Exception as e:
        logger.warning(f"Failed to load IMU entry point {ep.name!r}: {e}")

---

⚠️ Test isolation: roles dict is mutated in-place through get_mock()

tests/wrapper_test.py, lines 78, 401 and tests/devices_test.py, line 59

# wrapper_test.py:78
config.roles.update({IMUSensorTypes.mag: IMUDeviceID.IMU1})

# wrapper_test.py:401
config.roles.update({IMUSensorTypes.mag: IMUDeviceID.IMU0})

# devices_test.py:59
mutated_config.roles.update({IMUSensorTypes.mag: IMUDeviceID.IMU1})

get_mock() returns IMU_DEVICES[MOCK_NAME] directly. Calling .roles.update() on the returned config mutates the live registry entry, because roles is a dict and dataclasses.replace() does a shallow copy (so mutated_config.roles is config.roles is True). These tests bleed state into each other depending on execution order.

Fix: use replace(config, roles={**config.roles, ...}) or copy.deepcopy(config) before mutating.

Fix this →

---

🔧 Minor: get_mock() has a pointless try/except

src/imu_python/devices.py, lines 18–23

def get_mock() -> tuple[str, IMUConfig]:
    mock_name = MOCK_NAME
    try:
        return mock_name, IMU_DEVICES[mock_name]
    except KeyError:
        raise  # adds nothing

Simplify to:

def get_mock() -> tuple[str, IMUConfig]:
    """Return the MOCK IMU name and IMUConfig."""
    return MOCK_NAME, IMU_DEVICES[MOCK_NAME]

---

🔧 Minor: Stale module docstring in devices.py

src/imu_python/devices.py, line 1

"""Enum registry of IMU device configurations."""

The enum is gone. Update to e.g. """Functions for looking up IMU device configurations from the registry."""

---

🔧 Minor: _load_registry is missing a docstring

src/imu_python/registry.py, line 14 — per project conventions, all functions (public and private) should have docstrings.

---

🔧 Minor: _from_address iterates with key then re-indexes

src/imu_python/devices.py, lines 32–33

for device in IMU_DEVICES:
    base_config = IMU_DEVICES[device]

Prefer .items():

for device, base_config in IMU_DEVICES.items():

---

🔧 Minor: Test assertion is always truthy

tests/devices_test.py, line 15

assert (len(a.addresses) == 2 for a in IMU_DEVICES[device].devices.values())

This asserts a generator object (always truthy), not the result of evaluating it. Use all():

assert all(len(a.addresses) == 2 for a in IMU_DEVICES[device].devices.values())

---

✅ What looks good

• The two-group design (imu_module.devices for additions + imu_module.device_overrides for patching) is clean and the load order is correct
• reload_registry() in-place update pattern is safe and works correctly for the single-import use case
• builtin_devices.py cleanly resolves the circular import concern
• registry_test.py covers the override scenario with a proper mock
• get_config merging logic for split IMUs (LSM6DSOX+LIS3MDL) is unchanged and correct

[NGierden]

@claude-review, can you summarize me what has been added or changed in this PR?

[claude]

Claude finished @NGierden's task in 54s —— View job

---

PR Summary

• Read changed files
• Compile and post summary

---

This PR replaces the static IMUDevices enum with an extensible, entry-point-driven registry. Here's what changed:

---

New: registry.py

A new module that owns IMU_DEVICES: dict[str, IMUConfig] — the central device registry. At import time it calls _load_registry(), which:

1. Registers the four built-in sensors (MOCK, BNO055, BNO08X, LSM6DSOX_LIS3MDL) directly.
2. Scans the imu_module.devices entry point group to load any additional sensors registered by downstream packages.
3. Scans imu_module.device_overrides to let downstream packages replace an existing config (e.g. change a filter gain or sensor range) without forking.

A reload_registry() function is exposed for re-populating the dict at runtime if needed.

---

New: builtin_devices.py

The four IMUConfig instances (BNO055, BNO08X, LSM6DSOX_LIS3MDL, MOCK) that previously lived inside the IMUDevices enum were extracted into this new file. The split is needed to avoid a circular import (registry.py needs the configs, devices.py needs the registry).

---

Changed: devices.py (−278 / +88 lines)

The IMUDevices enum class is gone. It's replaced by three standalone functions that query IMU_DEVICES from the registry:

Old (enum methods)	New (module-level functions)
IMUDevices.get_mock()	get_mock() → tuple[str, IMUConfig]
IMUDevices._from_address(addr)	_from_address(addr)
IMUDevices.get_config(addresses)	get_config(addresses)

The logic inside these functions is unchanged — they do the same address-matching and partial-config merging for split IMUs (LSM6DSOX+LIS3MDL) as before.

---

Changed: definitions.py (+2 lines)

• Added MOCK_NAME = "MOCK" constant (used as the dict key for the mock sensor).
• Added IMUDescriptor dataclass (name: str, index: int) to replace enum-based device identification in the detected-device dict.

---

Changed: pyproject.toml

• Version bumped: 0.1.0 → 0.1.1.
• Two new entry point groups added:

  [project.entry-points."imu_module.devices"]
  # downstream packages declare new IMU configs here
  
  [project.entry-points."imu_module.device_overrides"]
  # downstream packages override existing IMU configs here

---

Changed: Tests

All tests updated to use IMU_DEVICES[name] and the new helper functions instead of IMUDevices.<MEMBER>. A new registry_test.py is added, covering:

• Built-in devices load correctly as IMUConfig instances.
• Known devices (e.g. MOCK) are present in the registry.
• The override mechanism replaces a built-in config when a matching entry point is present (tested with unittest.mock).

---

Why this matters

Previously, adding a new IMU or tweaking a sensor config required modifying devices.py in this repo directly. With this change, any downstream package (e.g. exosuit-python) can register a new sensor or override an existing one just by adding entries to its own pyproject.toml — no fork needed.

Note: There are two bugs and several minor issues flagged in the earlier review comment above that should be addressed before merging.

[NGierden]

Actually, I don't think I have anything to add:) Please address claudes' comments and then you're ready to merge.