The state
arm101/hardware/bus.py is 2449 lines holding three classes and ~80 methods:
|
lines |
what it is |
OverloadError |
415– |
the error type |
MotorBus |
465– |
the abstract contract |
FeetechBus |
845– |
the real serial implementation |
FakeBus |
1773–2449 |
a test double — 677 lines, 28% of the file — shipping inside the runtime package |
Plus the register map, the encoding helpers (encode_offset / decode_offset), the retry policy constants, and the EEPROM-settle constants, all at module scope.
Why now
Three separate hardware findings in the last two days each landed as another dense docstring in this one file, because there is nowhere else for them to go:
Each is correct and each is load-bearing. Together they have made the single most safety-critical file in the repo the hardest one to read.
The specific smell that prompted this
FakeBus is a test double in the shipped wheel. It exists because the runtime package has zero third-party dependencies and we wanted offline development — a good reason — but the consequence is that import arm101.hardware.bus pulls 677 lines of simulation into every production import, and the real/fake split is invisible at the module boundary.
It has also now demonstrably failed at its job: FakeBus/ServoModelBus could not model the reported-frame goal comparison, so an entire class of bug (the stale-goal lurch, #47) was invisible to the whole suite until tests/_rolling_servo.py::RollingServoBus was written. A fake that cannot reproduce the hardware's failure modes is not protecting the code that depends on it.
Suggested shape (not prescriptive)
bus/registers.py — the address map, widths, the EEPROM/SRAM boundary.bus/encoding.py — encode_offset/decode_offset, the sign-magnitude rules, load_magnitude, is_overload.bus/protocol.py — MotorBus (the contract) + OverloadError.bus/feetech.py — the real serial bus and its retry/lock/settle policy.bus/fake.py — the double. Consider moving it to tests/ and making the offline path an explicit, opt-in import, so the runtime never ships a simulation.
Whatever the split, the invariants must survive as tests, not as prose: tests/test_eeprom_limit_write_guard.py already pins the writable-register set ({5, 6, 31, 40, 41, 42, 46, 48, 55}) and the "no addr 9/11, ever" rule by parsing the module and resolving every write address statically. That test is the safety net that makes this refactor tractable — it will catch a register that moves or a write that appears.
The new asymmetry to preserve
write_offset now retries, verified by read-back — the only write in the file that retries at all. Every other write retries nothing, deliberately (#21, #38). That distinction is subtle and easy to flatten in a refactor: a blind write retry is how the Lock bug arose; a verified retry (read the register back, repeat only what the read proves is absent) is safe. Keep them visibly different.
Not urgent, and deliberately not now
Filed rather than done because it surfaced mid-hardware-run with the arm powered and PR #48 green. Restructuring the most safety-critical file in the repo at that moment would make a new bug indistinguishable from the one being chased.
Related: #21, #38, #43, #47
The state
arm101/hardware/bus.pyis 2449 lines holding three classes and ~80 methods:OverloadErrorMotorBusFeetechBusFakeBusPlus the register map, the encoding helpers (
encode_offset/decode_offset), the retry policy constants, and the EEPROM-settle constants, all at module scope.Why now
Three separate hardware findings in the last two days each landed as another dense docstring in this one file, because there is nowhere else for them to go:
result=-6on addr 31 that applied — see below).Each is correct and each is load-bearing. Together they have made the single most safety-critical file in the repo the hardest one to read.
The specific smell that prompted this
FakeBusis a test double in the shipped wheel. It exists because the runtime package has zero third-party dependencies and we wanted offline development — a good reason — but the consequence is thatimport arm101.hardware.buspulls 677 lines of simulation into every production import, and the real/fake split is invisible at the module boundary.It has also now demonstrably failed at its job:
FakeBus/ServoModelBuscould not model the reported-frame goal comparison, so an entire class of bug (the stale-goal lurch, #47) was invisible to the whole suite untiltests/_rolling_servo.py::RollingServoBuswas written. A fake that cannot reproduce the hardware's failure modes is not protecting the code that depends on it.Suggested shape (not prescriptive)
bus/registers.py— the address map, widths, the EEPROM/SRAM boundary.bus/encoding.py—encode_offset/decode_offset, the sign-magnitude rules,load_magnitude,is_overload.bus/protocol.py—MotorBus(the contract) +OverloadError.bus/feetech.py— the real serial bus and its retry/lock/settle policy.bus/fake.py— the double. Consider moving it totests/and making the offline path an explicit, opt-in import, so the runtime never ships a simulation.Whatever the split, the invariants must survive as tests, not as prose:
tests/test_eeprom_limit_write_guard.pyalready pins the writable-register set ({5, 6, 31, 40, 41, 42, 46, 48, 55}) and the "no addr 9/11, ever" rule by parsing the module and resolving every write address statically. That test is the safety net that makes this refactor tractable — it will catch a register that moves or a write that appears.The new asymmetry to preserve
write_offsetnow retries, verified by read-back — the only write in the file that retries at all. Every other write retries nothing, deliberately (#21, #38). That distinction is subtle and easy to flatten in a refactor: a blind write retry is how the Lock bug arose; a verified retry (read the register back, repeat only what the read proves is absent) is safe. Keep them visibly different.Not urgent, and deliberately not now
Filed rather than done because it surfaced mid-hardware-run with the arm powered and PR #48 green. Restructuring the most safety-critical file in the repo at that moment would make a new bug indistinguishable from the one being chased.
Related: #21, #38, #43, #47