Add Renode emulator driver for embedded target simulation#533
Open
vtz wants to merge 4 commits intojumpstarter-dev:mainfrom
Open
Add Renode emulator driver for embedded target simulation#533vtz wants to merge 4 commits intojumpstarter-dev:mainfrom
vtz wants to merge 4 commits intojumpstarter-dev:mainfrom
Conversation
❌ Deploy Preview for jumpstarter-docs failed. Why did it fail? →
|
Contributor
📝 WalkthroughWalkthroughAdds a new Renode driver package to Jumpstarter: async telnet monitor client, Renode power/flasher drivers, composite Renode driver and client CLI, extensive unit tests, package metadata, examples, README, ADR documenting design decisions and risks. Changes
Sequence DiagramssequenceDiagram
participant CLI
participant RenodeDriver as Renode Driver
participant Power as RenodePower
participant Monitor as RenodeMonitor
participant RenodeProc as Renode Process
participant PTY as PTY Terminal
CLI->>RenodeDriver: on()
RenodeDriver->>Power: on()
Power->>Power: pick free port
Power->>RenodeProc: spawn renode --monitor-port=<port>
Power->>Monitor: connect(host=localhost, port)
Monitor->>RenodeProc: TCP connection
Monitor->>Monitor: read until prompt
Monitor-->>Power: connected
Power->>Monitor: execute("machine LoadPlatform ...")
Monitor->>RenodeProc: send command + newline
Monitor->>Monitor: read until prompt
Monitor-->>Power: response
Power->>PTY: create PTY for UART
Power->>Monitor: execute("connector ...")
Monitor-->>Power: connector ok
alt firmware configured
Power->>Monitor: execute("sysbus LoadELF <path>")
Monitor-->>Power: load ok
end
Power->>Monitor: execute("start")
Monitor-->>Power: running
Power-->>RenodeDriver: on() complete
RenodeDriver-->>CLI: Ready
sequenceDiagram
participant CLI
participant RenodeDriver as Renode Driver
participant Flasher as RenodeFlasher
participant Monitor as RenodeMonitor
participant RenodeProc as Renode Process
CLI->>RenodeDriver: flash(firmware_resource)
RenodeDriver->>Flasher: flash(resource)
Flasher->>Flasher: write resource -> temp file
Flasher->>Flasher: record firmware path & load_command
alt simulation running
Flasher->>Monitor: execute("sysbus LoadELF <path>")
Monitor->>RenodeProc: perform load
Monitor-->>Flasher: success
Flasher->>Monitor: execute("machine Reset")
Monitor-->>Flasher: reset ok
end
Flasher-->>RenodeDriver: flash() complete
RenodeDriver-->>CLI: Firmware stored/loaded
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/client.py (1)
22-31: Accept the remainder of the CLI line as the monitor command.Most Renode monitor commands contain spaces, so this only works naturally when the whole command is shell-quoted. Using a variadic Click argument makes
... monitor sysbus LoadELF@foo.elf`` behave as expected.Proposed fix
def cli(self): base = super().cli() `@base.command`(name="monitor") - `@click.argument`("command") - def monitor_command(command): + `@click.argument`("command", nargs=-1) + def monitor_command(command: tuple[str, ...]): """Send a command to the Renode monitor.""" - result = self.monitor_cmd(command) + result = self.monitor_cmd(" ".join(command)) if result.strip(): click.echo(result.strip())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/client.py` around lines 22 - 31, The CLI monitor handler currently defines a single-word argument and should accept the remainder of the CLI line; update the Click argument in cli() for the nested monitor_command (the `@base.command` handler) to use a variadic argument (click.argument("command", nargs=-1)) and then join the tuple into a single string before calling monitor_cmd (e.g., command_str = " ".join(command) and pass command_str to self.monitor_cmd) so multi-word Renode monitor commands work without shell-quoting.python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py (1)
298-310: Consider renaming test for clarity.The test name
test_power_close_calls_offis misleading sinceclose()doesn't actually calloff()— it directly terminates the process without sending thequitcommand or disconnecting the monitor. Consider renaming totest_power_close_terminates_processto accurately reflect the behavior being tested.Suggested rename
`@pytest.mark.anyio` - async def test_power_close_calls_off(self): - """close() terminates the process.""" + async def test_power_close_terminates_process(self): + """close() terminates the process directly without monitor cleanup.""" driver = _make_driver()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py` around lines 298 - 310, Rename the test function to reflect actual behavior: change the test named test_power_close_calls_off to test_power_close_terminates_process and update its reference in the file (the async test function that calls RenodePower.close and asserts mock_process.terminate was called and power._process is None) so the name accurately describes that close() terminates the process rather than calling off().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.py`:
- Around line 99-127: The block that starts the Renode process with Popen and
then performs monitor setup (Popen -> self._process, RenodeMonitor,
self._monitor.connect, and subsequent self._monitor.execute calls) must be
wrapped in a try/except so any exception during monitor connection or commands
will teardown the spawned subprocess to avoid leaking it and leaving
self._process set. Modify the code so after creating self._process you use try:
... to run the RenodeMonitor connect and all execute calls (including load and
start), and in except Exception as e: ensure you cleanly stop the subprocess
(call self._process.terminate()/kill(), wait() and close pipes), set
self._process = None, then re-raise the exception; reference the symbols Popen,
self._process, RenodeMonitor, self._monitor.connect, self._monitor.execute and
ensure cleanup mirrors what off() would do.
- Around line 98-99: The Popen invocation in driver.py is piping
stdin/stdout/stderr (self._process = Popen(..., stdin=PIPE, stdout=PIPE,
stderr=PIPE)) which are never read and can cause the Renode process to block;
change those three arguments to use subprocess.DEVNULL instead so the child
won't deadlock (import DEVNULL if needed) — update the Popen call where
self._process is created and ensure any related code expecting piped streams is
removed or adjusted.
- Around line 144-148: The shutdown path is blocking because it calls the
synchronous Popen.wait(timeout=5) (used alongside self._process.terminate() and
except TimeoutExpired -> self._process.kill()) inside an async context (off());
change this to a non-blocking approach by running the blocking wait/killing
logic off the event loop (e.g., wrap the wait/kill sequence in a thread via
asyncio.to_thread or AnyIO's thread-runner) or replace it with an async poll
loop using self._process.poll() plus async sleep and then call kill if still
running; update the code around self._process.terminate(), self._process.wait(),
TimeoutExpired, and self._process.kill() accordingly so off() never blocks the
event loop.
In
`@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/monitor.py`:
- Around line 43-52: The execute method currently returns raw Renode responses;
update execute (in jumpstarter_driver_renode.monitor.Monitor.execute) to inspect
the text returned by _read_until_prompt and, if it indicates a command failure
(e.g., contains "ERROR", "Error", "Failed", or matches Renode's error prefix),
raise RenodeMonitorError with the full response instead of returning it; keep
logging the request/response but throw RenodeMonitorError to stop initialization
(adjust tests like test_monitor_execute_error_response and callers such as
RenodePower.on which rely on execute's behavior to handle failures).
---
Nitpick comments:
In
`@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/client.py`:
- Around line 22-31: The CLI monitor handler currently defines a single-word
argument and should accept the remainder of the CLI line; update the Click
argument in cli() for the nested monitor_command (the `@base.command` handler) to
use a variadic argument (click.argument("command", nargs=-1)) and then join the
tuple into a single string before calling monitor_cmd (e.g., command_str = "
".join(command) and pass command_str to self.monitor_cmd) so multi-word Renode
monitor commands work without shell-quoting.
In
`@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py`:
- Around line 298-310: Rename the test function to reflect actual behavior:
change the test named test_power_close_calls_off to
test_power_close_terminates_process and update its reference in the file (the
async test function that calls RenodePower.close and asserts
mock_process.terminate was called and power._process is None) so the name
accurately describes that close() terminates the process rather than calling
off().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d349f643-86a8-49ef-8781-30c12c5cb166
📒 Files selected for processing (12)
python/docs/source/contributing/adr/0001-renode-integration.mdpython/docs/source/contributing/adr/index.mdpython/docs/source/reference/package-apis/drivers/renode.mdpython/packages/jumpstarter-driver-renode/.gitignorepython/packages/jumpstarter-driver-renode/README.mdpython/packages/jumpstarter-driver-renode/examples/exporter.yamlpython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/__init__.pypython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/client.pypython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.pypython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.pypython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/monitor.pypython/packages/jumpstarter-driver-renode/pyproject.toml
python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.py
Outdated
Show resolved
Hide resolved
python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.py
Outdated
Show resolved
Hide resolved
python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.py
Show resolved
Hide resolved
python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/monitor.py
Show resolved
Hide resolved
ambient-code bot
pushed a commit
that referenced
this pull request
Apr 9, 2026
- Add structured Design Decisions (DD-N) section to JEP template following the ADR pattern used in the project (e.g., ADR-0001 from PR #533) - Add Consequences section (positive/negative/risks) to JEP template - Mark all template sections as mandatory, optional, or conditional - Document the relationship between JEPs and ADRs in JEP-0000 - Add SpecKit and ADR references to Prior Art in JEP-0000 - Add agent instructions and document conventions to jeps/README.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Introduce jumpstarter-driver-renode, a composite driver that enables Renode-based virtual hardware targets in Jumpstarter. Users can define any Renode-supported platform (STM32, S32K, Nucleo H753ZI, etc.) via exporter YAML configuration without modifying driver code. Key components: - RenodeMonitor: async telnet client for the Renode monitor interface - RenodePower: manages Renode process lifecycle and simulation control - RenodeFlasher: firmware loading via sysbus LoadELF / LoadBinary - RenodeClient: composite client with CLI extension for monitor commands Includes ADR-0001 documenting architectural decisions (control interface, UART exposure, configuration model, firmware loading strategy). Made-with: Cursor
- Replace PIPE with DEVNULL in Popen to prevent deadlocks (pipes were never read) - Wrap monitor setup in try-except to teardown subprocess on failure, preventing process leaks - Use anyio.to_thread.run_sync for blocking wait() in off() to avoid blocking the event loop - Raise RenodeMonitorError on error responses instead of silently returning error text - Accept multi-word monitor commands in CLI via nargs=-1 - Rename test_power_close_calls_off to test_power_close_terminates_process - Add docstrings across all public APIs Made-with: Cursor
Restructure the Renode integration ADR to follow the JEP template format: metadata table, Abstract, Motivation, Rejected Alternatives, Prior Art, Implementation History, and References sections. The DD-N design decisions and Consequences sections were already aligned. Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.py (2)
40-72: Simplify the running-state check inflash().Line 62 uses
hasattr(self.parent.children["power"], "_process"), but_processis always defined as a dataclass field withdefault=None. This check always returnsTrue, making it misleading. The effective guard isif monitor is not Noneon line 64.Consider replacing with a more explicit check:
Proposed fix
- if hasattr(self.parent.children["power"], "_process"): - monitor = self.parent.children["power"]._monitor - if monitor is not None: + power = self.parent.children["power"] + if power._process is not None and power._monitor is not None: + monitor = power._monitor + if True: # already checked aboveOr more simply:
- if hasattr(self.parent.children["power"], "_process"): - monitor = self.parent.children["power"]._monitor - if monitor is not None: + monitor = self.parent.children["power"]._monitor + if monitor is not None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.py` around lines 40 - 72, The hasattr(self.parent.children["power"], "_process") check in RenodeFlasher.flash is misleading because _process is always present; remove that hasattr guard and instead explicitly ensure the power child and its monitor exist before hot-loading: fetch the power child (e.g. power = self.parent.children.get("power")), check power is not None and power._monitor is not None, then assign monitor = power._monitor and run monitor.execute(...) as currently written; update the RenodeFlasher.flash method to use these explicit checks (referencing RenodeFlasher.flash, self.parent.children["power"], _monitor and _process to locate the code).
166-177: Monitor socket not explicitly closed in synchronousclose().The
close()method setsself._monitor = Nonewithout closing the underlying TCP socket. While the socket will likely be broken when Renode terminates, it's not explicitly cleaned up.Since
close()must be synchronous (per the learning, it's called during driver teardown outside an async context) anddisconnect()is async, a potential workaround is to directly close the socket if available:Possible enhancement
def close(self): """Synchronous cleanup for use during driver teardown.""" if self._process is not None: if self._monitor is not None: + # Best-effort sync close of the socket + if self._monitor._stream is not None: + try: + self._monitor._stream._transport.close() + except Exception: + pass self._monitor = NoneThis is a minor improvement — the socket will be cleaned up when Renode terminates anyway, but explicit cleanup is cleaner.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.py` around lines 166 - 177, The close() method currently nulled out self._monitor without closing the underlying socket; modify close() to synchronously close the monitor if present (e.g., call self._monitor.close() or the monitor's appropriate close/shutdown method) inside a try/except to swallow/LOG any exceptions, then set self._monitor = None, and proceed to terminate the process as before; reference the close() method and the self._monitor attribute (and keep disconnect() untouched since it is async).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py`:
- Around line 318-398: The async iterator mocks in TestRenodeFlasher tests
(test_flash_stores_firmware_path, test_flash_while_running_sends_load_and_reset,
test_flash_custom_load_command) use AsyncMock(return_value=iter(...)) which
doesn't implement async iteration; update each patched flasher.resource mock
(the mock_res used in those with patch.object(flasher, "resource")) to implement
async iteration by assigning mock_res.__aiter__ = lambda self: self and
mock_res.__anext__ = AsyncMock(side_effect=[<chunks...>, StopAsyncIteration()])
so the async for chunk in res: loops in RenodeFlasher.flash() iterate correctly
and yield the desired byte chunks.
---
Nitpick comments:
In
`@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.py`:
- Around line 40-72: The hasattr(self.parent.children["power"], "_process")
check in RenodeFlasher.flash is misleading because _process is always present;
remove that hasattr guard and instead explicitly ensure the power child and its
monitor exist before hot-loading: fetch the power child (e.g. power =
self.parent.children.get("power")), check power is not None and power._monitor
is not None, then assign monitor = power._monitor and run monitor.execute(...)
as currently written; update the RenodeFlasher.flash method to use these
explicit checks (referencing RenodeFlasher.flash, self.parent.children["power"],
_monitor and _process to locate the code).
- Around line 166-177: The close() method currently nulled out self._monitor
without closing the underlying socket; modify close() to synchronously close the
monitor if present (e.g., call self._monitor.close() or the monitor's
appropriate close/shutdown method) inside a try/except to swallow/LOG any
exceptions, then set self._monitor = None, and proceed to terminate the process
as before; reference the close() method and the self._monitor attribute (and
keep disconnect() untouched since it is async).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 381d75b0-27c9-41e5-9df3-3869a5a95ad9
⛔ Files ignored due to path filters (1)
python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
python/docs/source/contributing/adr/0001-renode-integration.mdpython/docs/source/contributing/adr/index.mdpython/docs/source/reference/package-apis/drivers/renode.mdpython/packages/jumpstarter-driver-renode/.gitignorepython/packages/jumpstarter-driver-renode/README.mdpython/packages/jumpstarter-driver-renode/examples/exporter.yamlpython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/__init__.pypython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/client.pypython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.pypython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.pypython/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/monitor.pypython/packages/jumpstarter-driver-renode/pyproject.toml
✅ Files skipped from review due to trivial changes (7)
- python/docs/source/reference/package-apis/drivers/renode.md
- python/packages/jumpstarter-driver-renode/.gitignore
- python/docs/source/contributing/adr/index.md
- python/packages/jumpstarter-driver-renode/examples/exporter.yaml
- python/packages/jumpstarter-driver-renode/pyproject.toml
- python/packages/jumpstarter-driver-renode/README.md
- python/docs/source/contributing/adr/0001-renode-integration.md
🚧 Files skipped from review as they are similar to previous changes (1)
- python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/client.py
python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py
Show resolved
Hide resolved
Contributor
Author
|
@kirkbrauer ADR-0001 has been restructured to align with the JEP template format from #423 (metadata table, Abstract, Motivation, Rejected Alternatives, Prior Art, Implementation History, References, and license footer). The DD-N design decisions and Consequences sections were already in the right shape. |
Replace incorrect AsyncMock(return_value=iter([...])) with idiomatic __aiter__/__anext__ pattern. The old pattern silently yielded nothing (with RuntimeWarning), the new one properly iterates the chunks. Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py (1)
241-248: Strengthen idempotency tests with observable assertions.Both tests currently pass on “no exception” only. Add explicit assertions that no startup/shutdown side effects were triggered (or assert expected warning logs), so regressions are caught deterministically.
🧪 Suggested direction
`@pytest.mark.anyio` async def test_power_on_idempotent(self): """Second on() call logs warning and does nothing.""" driver = _make_driver() power: RenodePower = driver.children["power"] power._process = MagicMock() - await power.on() + with patch("jumpstarter_driver_renode.driver._find_renode") as mock_find_renode: + await power.on() + mock_find_renode.assert_not_called() `@pytest.mark.anyio` async def test_power_off_idempotent(self): """Second off() call logs warning and does nothing.""" driver = _make_driver() power: RenodePower = driver.children["power"] power._process = None - await power.off() + await power.off() # no-op path + assert power._process is NoneAs per coding guidelines:
python/packages/jumpstarter-driver-*/jumpstarter_driver_*/driver_test.py: “Each driver package must include comprehensive tests indriver_test.py.”Also applies to: 290-297
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py` around lines 241 - 248, The idempotency tests (e.g., test_power_on_idempotent and the corresponding power.off test around lines 290-297) currently only await calls; enhance them to assert observable behavior: verify that RenodePower._process (accessed via power._process) was not called when calling power.on() or power.off() a second time, and assert that the expected warning log was emitted (mock the logger or use caplog) to ensure the function logged a warning rather than performing startup/shutdown side effects; update both tests to include these assertions so regressions are caught deterministically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py`:
- Around line 445-449: The test test_renode_pty_path should use
pathlib.Path-based assertions to mirror the driver implementation: replace the
string suffix check and manual substring check with a single Path
equality/assertion that Path(driver._pty) == Path(driver._tmp_dir.name) / "pty"
(or assert Path(driver._pty).resolve() == (Path(driver._tmp_dir.name) /
"pty").resolve()) so the test uses the same Path construction as the driver
(referencing driver._pty and driver._tmp_dir).
---
Nitpick comments:
In
`@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py`:
- Around line 241-248: The idempotency tests (e.g., test_power_on_idempotent and
the corresponding power.off test around lines 290-297) currently only await
calls; enhance them to assert observable behavior: verify that
RenodePower._process (accessed via power._process) was not called when calling
power.on() or power.off() a second time, and assert that the expected warning
log was emitted (mock the logger or use caplog) to ensure the function logged a
warning rather than performing startup/shutdown side effects; update both tests
to include these assertions so regressions are caught deterministically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 91eeec4c-c940-43e5-87c4-52dfc245331c
📒 Files selected for processing (1)
python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py
Comment on lines
+445
to
+449
| def test_renode_pty_path(self): | ||
| """PTY path is inside the temp directory.""" | ||
| driver = _make_driver() | ||
| assert driver._pty.endswith("/pty") | ||
| assert driver._tmp_dir.name in driver._pty |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether CI runs Python jobs on Windows (portability relevance check).
fd -i '.*\.(yml|yaml)$' .github/workflows \
| xargs rg -n -C2 'runs-on:\s*windows|windows-latest|matrix:|os:'Repository: jumpstarter-dev/jumpstarter
Length of output: 3429
🏁 Script executed:
# Check the driver implementation to understand _pty and platform specifics
cat -n python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.py | head -100Repository: jumpstarter-dev/jumpstarter
Length of output: 4103
🏁 Script executed:
# Search for _pty initialization and usage
rg -n '_pty' python/packages/jumpstarter-driver-renode/Repository: jumpstarter-dev/jumpstarter
Length of output: 855
🏁 Script executed:
# Check for platform-specific imports or guards
rg -n 'import os|import sys|import platform|sys.platform|platform.system' python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 53
🏁 Script executed:
# Check if there's any Windows-related logic or comments
rg -n -i 'windows|win32|win64|msvc|pty|tty|posix' python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 361
Use Path-based assertions for consistency with the implementation.
The test hardcodes /pty for string comparison, while the driver implementation (line 221-222 in driver.py) constructs the path using Path(self._tmp_dir.name) / "pty". The test assertions should mirror the implementation's approach for consistency.
Note: The Renode driver is POSIX-only since PTY (pseudo-terminal) is not a Windows feature, so this is a code style concern rather than a cross-platform portability issue.
🔧 Proposed fix
def test_renode_pty_path(self):
"""PTY path is inside the temp directory."""
driver = _make_driver()
- assert driver._pty.endswith("/pty")
- assert driver._tmp_dir.name in driver._pty
+ pty_path = Path(driver._pty)
+ assert pty_path.name == "pty"
+ assert driver._tmp_dir.name in str(pty_path.parent)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@python/packages/jumpstarter-driver-renode/jumpstarter_driver_renode/driver_test.py`
around lines 445 - 449, The test test_renode_pty_path should use
pathlib.Path-based assertions to mirror the driver implementation: replace the
string suffix check and manual substring check with a single Path
equality/assertion that Path(driver._pty) == Path(driver._tmp_dir.name) / "pty"
(or assert Path(driver._pty).resolve() == (Path(driver._tmp_dir.name) /
"pty").resolve()) so the test uses the same Path construction as the driver
(referencing driver._pty and driver._tmp_dir).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
jumpstarter-driver-renode, a composite driver that enables Renode-based virtual hardware targets in JumpstarterRenodeMonitor(async telnet client),RenodePower(process lifecycle),RenodeFlasher(firmware loading viasysbus LoadELF/LoadBinary), andRenodeClient(composite client with CLI extension)Design Decisions
Key architectural decisions are documented in
python/docs/source/contributing/adr/0001-renode-integration.md:anyio.connect_tcpextra_commandsfor customizationflash()thenon()semanticFiles Changed
python/packages/jumpstarter-driver-renode/— Full driver package (monitor, driver, client, tests, examples)python/docs/source/contributing/adr/— ADR index and ADR-0001 for Renode integrationpython/docs/source/reference/package-apis/drivers/renode.md— Docs symlink to driver READMETest Plan
RenodeMonitor(connect, execute, close, timeout, error handling)RenodePower(on/off, firmware loading, UART configuration)RenodeFlasher(flash, format detection, not-running error)Renodeconfig validation (missing platform, defaults, UART config)serve()(requires Renode installed, marked withskipif)python-tests.yaml(follow-up)Made with Cursor