Mock flag

[CatYang3]

Summary

Describe the main changes in this PR:

• Added flag to use mock, so mock IMUs can be created on the Jetson;
• Added tests for the IMUFactory so that mocks are always used for testing.

---

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 1m 58s —— View job

---

Code Review — PR #86: Mock flag

Tasks:

• Load tools
• Read changed files
• Analyze diff and code quality
• Post review feedback

---

Summary

The PR adds a --mock / -m CLI flag and a create_mock parameter to IMUFactory so mock IMUs can be instantiated without real hardware. It also updates the existing factory test to use mock mode (making it runnable in CI) and adds two new tests for scan_i2c_bus. Overall the approach is clean, but there is one behavioral bug and a few smaller issues worth addressing.

---

Bug: Mock manager is created once per bus, yielding duplicates

Severity: Medium

detect_and_create() loops over all entries in I2CBusID (currently bus_1 and bus_7) and calls _detect_and_create_per_bus() for each. When create_mock=True, every call fetches the same mock addresses from get_mock() and returns the same manager — so the list ends up with 2 identical mock IMUManager instances for the price of one.

# factory.py:41-49
for bus in I2CBusID:          # iterates bus_1 AND bus_7
    i2c_lock = Lock()
    managers += IMUFactory._detect_and_create_per_bus(
        ...
        create_mock=create_mock,  # same mock returned each time
    )

The test only asserts len(imu_managers) > 0, so it doesn't catch this.

Suggested fix: Either exit the loop after the first bus in mock mode, or create the mocks outside the bus loop:

if create_mock:
    _, mock_cfg = get_mock()
    addresses = [a for d in mock_cfg.devices.values() for a in d.addresses]
    managers += IMUFactory._create_from_addresses(addresses, ...)
    return managers   # skip bus loop entirely

Alternatively, tighten the assertion in the test:

_, mock_cfg = get_mock()
expected_per_bus = len(get_config([a for d in mock_cfg.devices.values() for a in d.addresses]))
assert len(imu_managers) == expected_per_bus   # not n_buses × expected

Fix this →

---

Minor: JetsonBus.get() called unnecessarily in mock mode

In _detect_and_create_per_bus, the I2C bus is always initialized even when create_mock=True (where the result is immediately discarded):

# factory.py:81
i2c_bus = JetsonBus.get(bus_id=i2c_id)   # triggers hardware init

if create_mock:
    _, mock = get_mock()
    addresses = [...]    # i2c_bus not used

On non-Jetson hardware this causes harmless-but-noisy warning logs. Guarding the call would clean things up:

i2c_bus = None if create_mock else JetsonBus.get(bus_id=i2c_id)

Fix this →

---

Pre-existing: broad except Exception in scan_i2c_bus

Not introduced by this PR, but the new test_scan_i2c_bus_exception_returns_mock_addresses test highlights it. CLAUDE.md specifies catching OSError for I2C errors specifically:

# factory.py:126 — pre-existing
except Exception as err:   # should be OSError (I2C errors are OSError)

Worth fixing as a follow-up or in this PR since the test already documents the expected failure case as OSError.

---

Nits

• __main__.py:15: mock: bool parameter name shadows the standard library unittest.mock module name. Renaming to use_mock: bool at the main() signature level (not the argparse side) would be safer and more descriptive. Small, but worth noting given mock is imported from unittest.mock in test files.
• tests/factory_test.py:31: The comment # device may not exist if not reloaded is unclear — "not reloaded" has no obvious referent in the code.

---

What's good

• Cleanly threads create_mock through the factory call chain ✓
• --mock/-m CLI flag is natural and well-documented in argparse ✓
• Converting test_imu_factory to use create_mock=True makes it runnable in CI without hardware ✓
• Changed >= 0 to > 0 is a meaningful assertion improvement ✓
• test_scan_i2c_bus_success and test_scan_i2c_bus_exception_returns_mock_addresses cover the two main code paths ✓
• Docstring fix in i2c_bus.py (bus_1/bus_7 instead of stale left/right) ✓

---

[NGierden]

See claude comments

[CatYang3]

Addressing Claude's comments:

• Manager duplicates: for each bus, one manager with address 0x00 (index 0) and one manager with address 0x01 (index 1) are created. In total, 4 managers are created in mock mode. That's intended.
• JetsonBus unnecessary call: fixed in the last commit.
• mock param in __main__: fixed in the last commit.
• comment in factory_test: the comment was referring to the wrapper's reload function. Part of the test was never executed because the wrapper's device dict was always empty. In the last commit, I added a reload call to populate the dict so that the devices can be checked.

The code is ready to merge now in my opinion.