feat(arduino): add ArduinoModule with full arduino sim (and real arduino) support#1879
feat(arduino): add ArduinoModule with full arduino sim (and real arduino) support#1879jeff-hykin wants to merge 15 commits intodevfrom
Conversation
Greptile SummaryThis PR adds Confidence Score: 5/5Safe to merge; all findings are P2 style/polish suggestions with no impact on correctness or runtime behaviour. The implementation is thorough — shared DSP parser, file-locked bridge builds, proper QEMU teardown, typed-channel validation, AVR SRAM payload guards, and a three-way registry sync test. No P0/P1 bugs were found. The four P2 items (fcntl import guard, silent unconnected-stream skip, substring core-check, stale doc comment) and the committed generated file are all non-blocking improvements. dimos/core/arduino_module.py (fcntl import, unconnected-stream warning, core substring check), dimos/hardware/arduino/common/dsp_protocol.h (stale doc), examples/arduino_led_echo/sketch/dimos_arduino.h (generated file in repo) Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User Code
participant AM as ArduinoModule.build()
participant CLI as arduino-cli
participant Nix as nix build
participant AM2 as ArduinoModule.start()
participant QEMU as qemu-system-avr
participant Bridge as arduino_bridge (C++)
participant LCM as LCM Bus
User->>AM: build()
AM->>AM: _generate_header() → dimos_arduino.h
AM->>CLI: compile sketch (--build-path)
CLI-->>AM: ELF binary
AM->>Nix: nix build .#arduino_bridge (file-locked)
Nix-->>AM: result/bin/arduino_bridge
AM->>CLI: upload (physical) or skip (virtual)
User->>AM2: start()
AM2->>AM2: _resolve_topics() (validate # suffix)
AM2->>AM2: _build_topic_enum() (alphabetical IDs)
alt virtual=True
AM2->>QEMU: Popen(qemu-system-avr -machine uno -bios sketch.elf -serial pty)
QEMU-->>AM2: char device redirected to /dev/pts/N
AM2->>Bridge: Popen(arduino_bridge --serial_port /dev/pts/N --topic_out 1 ch#type ...)
else virtual=False
AM2->>Bridge: Popen(arduino_bridge --serial_port /dev/ttyACM0 ...)
end
Bridge->>LCM: subscribe(ch#type) for each In stream
LCM-->>Bridge: LCM message received
Bridge->>QEMU: DSP frame (START|TOPIC|LEN|PAYLOAD|CRC8)
QEMU->>Bridge: DSP frame (echo/sensor data)
Bridge->>LCM: publish(ch#type, fingerprint+payload)
Reviews (1): Last reviewed commit: "add arduino example (tested on hardware!..." | Re-trigger Greptile |
|
|
||
| from dataclasses import dataclass | ||
| import errno | ||
| import fcntl |
There was a problem hiding this comment.
Unconditional
fcntl import breaks Windows
fcntl is a POSIX-only module. Any Windows machine that imports this file (directly or via a transitive import) will get ModuleNotFoundError: No module named 'fcntl', even if it never creates an ArduinoModule. Since arduino_module.py may end up on a broad import path, guarding the import (or deferring it to the method body) prevents surprising failures on non-target platforms.
Or move the import fcntl inside _build_bridge so the error is deferred to the call site with a meaningful context.
| for stream_name, topic_id in topic_enum.items(): | ||
| if stream_name not in topics: | ||
| continue | ||
| lcm_channel = topics[stream_name] | ||
| if stream_name in self.outputs: | ||
| bridge_args.extend(["--topic_out", str(topic_id), lcm_channel]) | ||
| elif stream_name in self.inputs: | ||
| bridge_args.extend(["--topic_in", str(topic_id), lcm_channel]) |
There was a problem hiding this comment.
Silently skipped unconnected streams produce confusing bridge errors
When a stream has no LCM transport (not wired in the blueprint), it is absent from topics and silently omitted from the bridge's --topic_in/--topic_out args. The Arduino sketch will still send DSP frames on that topic ID (since the generated dimos_arduino.h always includes all topic IDs), so the bridge will log "Unknown outbound topic: N" at runtime — a confusing error with no explanation at build time.
A logger.warning for streams in topic_enum but not in topics would make the mismatch obvious during start():
for stream_name, topic_id in topic_enum.items():
if stream_name not in topics:
logger.warning(
"Stream has no LCM transport and will not be relayed by the bridge",
stream=stream_name,
topic_id=topic_id,
)
continue| ) | ||
| if list_result.returncode == 0 and core_id in list_result.stdout: |
There was a problem hiding this comment.
core_id in stdout substring check can produce false positives
core_id in list_result.stdout matches the string anywhere in the output, including as a prefix of a different core name (e.g., "arduino:avr" would match if "arduino:avr-plus" is installed). This could cause the install step to be skipped when the required core is absent, leading to a confusing compile failure later.
A line-aware check is more precise:
| ) | |
| if list_result.returncode == 0 and core_id in list_result.stdout: | |
| if list_result.returncode == 0 and any( | |
| line.split()[0] == core_id | |
| for line in list_result.stdout.splitlines() | |
| if line.strip() | |
| ): |
| * | ||
| * This file provides: | ||
| * - CRC-8/MAXIM table + computation | ||
| * - dimos_init(baud) | ||
| * - dimos_send(topic, data, len) | ||
| * - dimos_poll() |
There was a problem hiding this comment.
Stale API listing in header doc comment
The "This file provides" block lists dimos_poll() and dimos_on_receive(topic, handler) which do not exist in the file. The actual public API is dimos_check_message(), dimos_message_topic(), dimos_message_data(), and dimos_message_len().
| * | |
| * This file provides: | |
| * - CRC-8/MAXIM table + computation | |
| * - dimos_init(baud) | |
| * - dimos_send(topic, data, len) | |
| * - dimos_poll() | |
| * This file provides: | |
| * - CRC-8/MAXIM table + computation | |
| * - dimos_init(baud) | |
| * - dimos_send(topic, data, len) | |
| * - dimos_check_message() / dimos_message_topic() / dimos_message_data() / dimos_message_len() | |
| * - DimosSerial class (Serial.print shim → debug frames) |
| /* Auto-generated by DimOS ArduinoModule — do not edit */ | ||
| #ifndef DIMOS_ARDUINO_H | ||
| #define DIMOS_ARDUINO_H | ||
|
|
||
| /* --- Config --- */ | ||
| #define DIMOS_BAUDRATE 115200 | ||
|
|
||
| /* --- Topic enum (shared with C++ bridge) --- */ | ||
| enum dimos_topic { | ||
| DIMOS_TOPIC_DEBUG = 0, | ||
| DIMOS_TOPIC__LED_CMD = 1, /* In[Bool] */ | ||
| DIMOS_TOPIC__LED_STATE = 2, /* Out[Bool] */ | ||
| }; | ||
|
|
||
| /* --- Message type headers --- */ | ||
| #include "std_msgs/Bool.h" | ||
|
|
||
| /* --- DSP protocol core --- */ | ||
| #include "dsp_protocol.h" | ||
|
|
||
| #endif /* DIMOS_ARDUINO_H */ |
There was a problem hiding this comment.
Generated file committed outside the gitignore scope
The dimos/hardware/arduino/.gitignore entry covers examples/*/sketch/dimos_arduino.h (relative to dimos/hardware/arduino/), but this file lives at examples/arduino_led_echo/sketch/dimos_arduino.h at the repo root — a different path not covered by that rule. If the LedEcho module's streams or config change, this committed file will drift from what ArduinoModule._generate_header() would actually produce.
Consider adding examples/arduino_led_echo/sketch/dimos_arduino.h to the root .gitignore and removing the file, or adding a comment explaining why the committed copy is intentional for the hardware-tested demo.
|
pasting comment from Discord here: Will chat on this in person first, I think it's really cool to auto-build and deploy onto a microcontroller when your blueprint runs, we can even have a small version control protocol to not write firmware every time. I'd start with just establishing good comms, then do autobuilds etc separately. I see a bunch of messages are redefined here by hand due to malloc, but I think we should just write a small arduino compatible encoder/decoder on our dimos-lcm cpp side and not merge 6000 lines of mirrored messages into main repo. LCM proto also gives you pubsub out of the box (topic encoding, packets etc, super helpful for serial) we can disable hashing or change the algo if this is an issue - actually pretty important for serial comms on arduino side ideally we have the exact same cpp API, why can't I do something like (I'm completely vibing) |
| # mypy 3.10 reports ``__getstate__ undefined in superclass``. Use | ||
| # ``getattr`` with a default to bypass the static lookup; the | ||
| # result is typed ``Any`` which lets the rebind to ``state`` pass | ||
| # both mypy versions without a ``# type: ignore``. |
There was a problem hiding this comment.
this shouldnt be here
| "spatial-memory": "dimos.perception.spatial_perception.SpatialMemory", | ||
| "speak-skill": "dimos.agents.skills.speak_skill.SpeakSkill", | ||
| "temporal-memory": "dimos.perception.experimental.temporal_memory.temporal_memory.TemporalMemory", | ||
| "twist-echo": "dimos.hardware.arduino.examples.arduino_twist_echo.module.TwistEcho", |
There was a problem hiding this comment.
should be named different
| # us a dtype-parameterized return without a cast. `.astype` | ||
| # also always returns a fresh array, so the copy semantics are | ||
| # preserved. | ||
| return arr[:num_joints].astype(np.float64) |
Introduces a new DimOS module type for Arduino-based hardware with LCM- compatible binary serialization, a generic serial↔LCM bridge, and a QEMU-based virtual mode for testing sketches without real hardware. Core pieces: - lcm_coretypes_arduino.h: drop-in replacement for LCM's coretypes that compiles on AVR (no malloc, no variable-length, memcpy-safe float casts) - arduino_msgs/: C encode/decode for 20 fixed-size LCM types across std_msgs and geometry_msgs, wire-format identical to LCM C++ (minus the 8-byte fingerprint hash, which the bridge prepends/strips) - dsp_protocol.h: framed binary protocol [0xD1][topic][len][payload][crc8] with a direct USART driver (bypasses Arduino HardwareSerial to avoid its interrupt-driven TX, which doesn't fire in QEMU's AVR USART model) - arduino_bridge: generic C++ binary that maps topic_id <-> LCM channel via CLI args; one binary for all ArduinoModule instances - ArduinoModule: Python base class with build() RPC that generates dimos_arduino.h from stream declarations, compiles the sketch via arduino-cli, and builds the bridge. When virtual=True, start() launches qemu-system-avr with -serial pty and parses the PTY path from stderr. - flake.nix: builds arduino_bridge and provides arduino-cli + tools Tests: - test_wire_compat: C++ test that builds against the same dimos-lcm C++ headers as the bridge and verifies byte-level equivalence for all 20 message types + a roundtrip. 21/21 passing. - examples/arduino_twist_echo: end-to-end example with a Twist-echo sketch, a TestPublisher module, and a blueprint wiring them together. Verified: real DimOS coordinator + worker pool + LCM runs the sketch in QEMU and Twist flows round-trip through Python <-> bridge <-> DSP <-> virtual ATmega328P.
… any shell ArduinoModule previously shelled out to bare ``arduino-cli`` / ``avrdude`` / ``qemu-system-avr`` names, which only worked inside ``nix develop``. Running a blueprint from plain Python would fail with "arduino-cli not found" unless the user had already entered the flake shell — defeating the point of importing the module as a library. Package all three tools as a new ``dimos_arduino_tools`` flake output (a ``symlinkJoin`` over ``arduino-cli.pureGoPkg``, ``avrdude``, and ``pkgs.qemu``) and resolve it at runtime via ``nix build .#dimos_arduino_tools --print-out-paths --no-link``, cached per process. Only ``nix`` itself needs to be on PATH; the e2e test's skip-precheck is relaxed accordingly. Switch the dev shell from ``pkgs.arduino-cli`` (a bwrap FHS wrapper that fails when nested userns is restricted) to ``pkgs.arduino-cli.pureGoPkg`` (the unwrapped Go binary, which runs natively on any host with a real ``/lib64/ld-linux-x86-64.so.2`` or nix-ld enabled). Also fix a latent bug in ``_generate_header``: arduino-cli's sketch preprocessor doesn't honor ``-I`` paths passed via ``compiler.cpp.extra_flags``, so a header living outside the sketch dir was invisible during the preprocessing pass, breaking any sketch that referenced an enum from the generated header. Write ``dimos_arduino.h`` directly into the sketch dir (and gitignore it).
…odule - Annotate ``payload`` in ``test_detect_port_raises_on_no_match`` so mypy can infer the type of the empty inner list. - Extend the existing ``# type: ignore[method-assign]`` comments on the ``_make_module_with_topics`` property overrides to also cover ``assignment`` — property assignment to a non-property attribute trips both error codes, and the project's ``warn_unused_ignores`` is off so listing both is safe. These are not in the CI path (project mypy excludes ``test_*`` files) but they fail when mypy is run on the test file directly.
… API - New dimos_lcm_pubsub.h: transport-agnostic LCM pubsub engine (from dimos-lcm) - New dimos_lcm_serial.h: serial transport adapter wiring DSP + LCM pubsub - DSP protocol upgraded to 2-byte topic IDs (uint16_t, up to 65534 topics) - C++ bridge simplified: passthrough for fingerprinted payloads, 2-byte topics - ArduinoModule generates channel mapping table, type descriptors, channel constants - Example sketches use new API: dimos_subscribe() + dimos_handle() + typed callbacks - Arduino message headers updated with LCM fingerprint constants - Fixed flake.nix nixpkgs compatibility (pureGoPkg removed upstream) - All 23 unit tests pass, e2e virtual Arduino round-trip passes
…cho test - flake.nix: switch dimos_arduino_tools and devShell from bwrap-wrapped pkgs.arduino-cli to pkgs.arduino-cli.pureGoPkg, fixing "Permission denied" on hosts that block unshare(CLONE_NEWUSER) - New arduino_multi_echo example: echoes Bool, Int32, Vector3, and Quaternion through real hardware with float64→float32 precision checks - All tests pass on Arduino Uno (30/30 assertions, zero dropped messages)
…bility Uses 'pkgs.arduino-cli.pureGoPkg or pkgs.arduino-cli' so the flake works on both Linux (where pureGoPkg exists and bwrap breaks) and macOS (where pureGoPkg doesn't exist).
…doring Remove 55 vendored files (53 generated arduino_msgs/*.h + duplicate lcm_coretypes_arduino.h and dimos_lcm_pubsub.h) from dimos4. Headers are now sourced from dimos-lcm at build time via the dimos_arduino_tools nix package (share/arduino_msgs/). - flake.nix: pin dimos-lcm to jeff/feat/arduino, package generated headers into dimos_arduino_tools via runCommand - arduino_module.py: resolve message headers from nix store path instead of local common/arduino_msgs/ - CMakeLists.txt: point wire compat test at dimos-lcm headers - Tests: resolve headers via nix build (skip if nix unavailable) Reduces PR from 84 files to ~29 net changed files.
…th override - Use lcm-extended in test flake.nix (fixes fdatasync on macOS) - Replace ClassVar flag with overridable _build_topic_args() method
2594fe8 to
f586e6e
Compare
… in git - _encoded_payload_size now returns full wire size (fingerprint + data), fixing an off-by-8 in AVR SRAM validation - MAX_TOPICS updated from 255 to 65534 to match 2-byte topic IDs - Remove generated dimos_arduino.h from examples/ and add .gitignore
Cut ~244 lines of over-explained comments. Kept docstrings that add information beyond the method name; removed those that just restate it.
Add config fields for AVR compile-time knobs:
- max_subs, max_pending, max_msg_size, max_payload (None = use header
defaults; set to override via -D flags)
Add arduino_defines dict for arbitrary user #defines:
arduino_defines={"MOTOR_PIN": 13, "THRESHOLD": 0.5}
→ #define MOTOR_PIN 13 / #define THRESHOLD 0.5f
Both emitted in dimos_arduino.h AND passed as -D compiler flags.
SRAM validation now respects user's max_payload override.
EXPERIMENTAL / TOY / FUN
Problem
We don't have arduino support :(
Solution
Arduino support. Just like a native module, there's an Arduino module, but you give it an .ino file instead of a C++ file.
How does it work?
%%{init: {'theme':'base', 'themeVariables': {'fontSize': '40px', 'fontFamily': 'Inter, sans-serif'}}}%% flowchart LR subgraph Dimos["🖥️ dimos"] direction TB SrcModule["Some dimos module"] DstModule["Some other dimos module"] Terminal["📟 Terminalprintouts"] end subgraph Host["C++ Native Module"] direction TB Encode["Strip 64-bit LCM checksum"] Decode["Recreate 64-bit LCM checksum"] PrintHandler["Print handlerforward to terminal"] end subgraph Arduino["Arduino"] direction TB Parse["read serial, lcm_crappy decode (fixed size only)"] Logic["Arduino does its thing"] Emit["lcm_crappy encode and publish"] Printf["Serial.printpatched by dimos header"] Parse --> Logic --> Emit Logic -.-> Printf end SrcModule -->|"LCM message"| Encode Encode -->|"serial (data)"| Parse Emit -->|"serial (data)"| Decode Decode -->|"LCM message"| DstModule Printf -->|"serial (printout)"| PrintHandler PrintHandler -->|"stdout"| Terminal classDef dimos fill:#ede9fe,stroke:#6d28d9,stroke-width:2px,color:#1e1b4b classDef host fill:#d1fae5,stroke:#047857,stroke-width:2px,color:#064e3b classDef arduino fill:#1e3a8a,stroke:#1e40af,stroke-width:2px,color:#ffffff classDef terminal fill:#fef3c7,stroke:#b45309,stroke-width:2px,color:#78350f class Dimos,SrcModule,DstModule dimos class Terminal terminal class Host,Encode,Decode,PrintHandler host class Arduino,Parse,Logic,Emit,Printf arduinoBreaking ChangesCaveats
How to Test
Contributor License Agreement