esp32: introduce esp32 driver#294
Merged
mangelajo merged 3 commits intojumpstarter-dev:mainfrom Mar 12, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new Jumpstarter ESP32 driver package with docs, packaging, driver and client implementations, CLI commands, examples, and unit tests for flashing, dumping, erasing, bootloader entry, and chip queries via esptool. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as Esp32FlasherClient CLI
participant Client as Esp32FlasherClient
participant Driver as Esp32Flasher
participant Esptool as esptool
participant ESP32
User->>CLI: run flash command (firmware, --address)
CLI->>Client: call flash(...)
Client->>Driver: flash(source, target)
Driver->>Driver: _parse_region(target)
Driver->>Driver: stream source -> temp file
Driver->>Esptool: _connect_esp()
Esptool->>ESP32: open serial / handshake
ESP32-->>Esptool: respond
Esptool-->>Driver: connected
Driver->>Esptool: write_flash(address, data)
Esptool->>ESP32: program flash
ESP32-->>Esptool: ack
Esptool-->>Driver: success
Driver->>Driver: hard_reset()
Driver->>Esptool: _close_esp()
Driver-->>Client: return status
Client-->>CLI: print result
CLI-->>User: display completion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
python/packages/jumpstarter-driver-esp32/.gitignore (1)
1-3: Consider adding common Python .gitignore patterns.The current entries are correct, but you might want to include additional standard Python patterns to prevent accidentally committing build artifacts, test caches, and virtual environments.
📝 Suggested additions
__pycache__/ +*.py[cod] +*.so .coverage coverage.xml +htmlcov/ +.pytest_cache/ +*.egg-info/ +dist/ +build/ +.venv/ +venv/ +env/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-esp32/.gitignore` around lines 1 - 3, The .gitignore currently lists __pycache__/, .coverage and coverage.xml but misses other common Python ignore patterns; update the file to also exclude compiled bytecode (*.pyc, *.pyo), test and type-cache directories (.pytest_cache/, .mypy_cache/, .cache/), virtualenv folders (venv/, env/, .venv/), build/distribution outputs (build/, dist/, *.egg-info/, .eggs/, pip-wheel-metadata/), tox/ and wheel artifacts (*.egg, *.whl), and IDE/editor files (e.g., .vscode/, .idea/) so that files like compiled artifacts, virtual environments, test/type caches, build outputs and editor settings are not accidentally committed; keep the existing entries __pycache__/, .coverage and coverage.xml and append these common patterns.python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver_test.py (1)
106-113: Assert the DTR/RTS sequence, not justclose().This only verifies teardown, so a broken bootloader toggle order would still pass. Recording the attribute writes on
mock_serialwould make this test cover the behavior the method is meant to implement.As per coding guidelines, "Each driver package must include comprehensive tests in
driver_test.py".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver_test.py` around lines 106 - 113, The test currently only asserts mock_serial.close() and misses verifying the DTR/RTS toggle sequence performed by Esp32Flasher.enter_bootloader; update test_enter_bootloader to record attribute assignments on mock_serial (inspect mock_serial.mock_calls for __setattr__ entries for 'dtr' and 'rts') and assert that the sequence of (__setattr__, 'dtr', value) and (__setattr__, 'rts', value) calls matches the exact order and boolean values implemented by Esp32Flasher.enter_bootloader so the bootloader toggle behavior is covered.
🤖 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-esp32/jumpstarter_driver_esp32/client.py`:
- Around line 35-39: The subclass CLI override for flash replaces the base
FlasherClient.cli compression option with type=str, removing click-level
validation; update the `@click.option` for "--compression" in the flash function
so it uses the same Click choice constraint as the base (i.e. the same
click.Choice and default used by FlasherClient.cli) or import/replicate the
exact compression choices from the base implementation so invalid values are
rejected and the valid options show up in --help.
In `@python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.py`:
- Around line 36-44: The code always auto-detects the ESP32 in _connect_esp()
using esptool.cmds.detect_chip, ignoring the Esp32Flasher configuration value
self.chip; update _connect_esp to honor that config by: if self.chip is set, use
it instead of auto-detect (either pass the chip name into the esptool call if
the API supports a chip/chip_name parameter, or skip detect_chip and
construct/choose the appropriate esp descriptor based on self.chip), otherwise
fall back to esptool.cmds.detect_chip; reference Esp32Flasher, _connect_esp,
self.chip, and esptool.cmds.detect_chip when making the change (or remove the
public self.chip option and its docs if you prefer not to support manual chip
selection).
In `@python/packages/jumpstarter-driver-esp32/pyproject.toml`:
- Line 13: The Click dependency spec "click>=8.1.7.2" in pyproject.toml is
invalid (no such release); update the dependency line referencing Click to a
valid constraint such as "click>=8.1.7" (or pin to "click==8.1.7" if you need
exact version) so the package can be installed; edit the dependency entry in
python/packages/jumpstarter-driver-esp32/pyproject.toml to replace
"click>=8.1.7.2" with the corrected constraint.
- Around line 11-17: The package currently relies on pyserial transitively via
esptool but directly uses serial.Serial in
jumpstarter_driver_esp32.driver.enter_bootloader(), so add pyserial as an
explicit dependency in the pyproject.toml dependencies list for the
jumpstarter-driver-esp32 package (e.g., "pyserial>=X.Y" with a minimal
compatible version) to make the runtime requirement explicit and stable; update
the dependencies array where anyio/click/esptool are declared and run your
build/lock step to verify resolution.
In `@python/packages/jumpstarter-driver-esp32/README.md`:
- Around line 52-65: Update the README code fence to include the missing
"bootloader" subcommand and add a language tag to the fence; specifically modify
the CLI usage block (the fenced example showing j esp32 commands) to list
"bootloader" alongside chip-info, dump, erase, flash, and reset, and change the
opening fence from ``` to ```text or ```console so markdownlint recognizes the
language; this aligns the docs with the implementation in
jumpstarter_driver_esp32/client.py where the bootloader subcommand is defined.
- Around line 42-45: Update the autodoc member list for
jumpstarter_driver_esp32.client.Esp32FlasherClient to include the public method
enter_bootloader (add it to the :members: list alongside flash, dump,
get_chip_info, erase, hard_reset) so the API reference documents
Esp32FlasherClient.enter_bootloader.
---
Nitpick comments:
In `@python/packages/jumpstarter-driver-esp32/.gitignore`:
- Around line 1-3: The .gitignore currently lists __pycache__/, .coverage and
coverage.xml but misses other common Python ignore patterns; update the file to
also exclude compiled bytecode (*.pyc, *.pyo), test and type-cache directories
(.pytest_cache/, .mypy_cache/, .cache/), virtualenv folders (venv/, env/,
.venv/), build/distribution outputs (build/, dist/, *.egg-info/, .eggs/,
pip-wheel-metadata/), tox/ and wheel artifacts (*.egg, *.whl), and IDE/editor
files (e.g., .vscode/, .idea/) so that files like compiled artifacts, virtual
environments, test/type caches, build outputs and editor settings are not
accidentally committed; keep the existing entries __pycache__/, .coverage and
coverage.xml and append these common patterns.
In
`@python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver_test.py`:
- Around line 106-113: The test currently only asserts mock_serial.close() and
misses verifying the DTR/RTS toggle sequence performed by
Esp32Flasher.enter_bootloader; update test_enter_bootloader to record attribute
assignments on mock_serial (inspect mock_serial.mock_calls for __setattr__
entries for 'dtr' and 'rts') and assert that the sequence of (__setattr__,
'dtr', value) and (__setattr__, 'rts', value) calls matches the exact order and
boolean values implemented by Esp32Flasher.enter_bootloader so the bootloader
toggle behavior is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3a041102-45f4-4af5-8f77-c3dd8d195b2e
⛔ Files ignored due to path filters (1)
python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
python/docs/source/reference/package-apis/drivers/esp32.mdpython/docs/source/reference/package-apis/drivers/index.mdpython/packages/jumpstarter-driver-esp32/.gitignorepython/packages/jumpstarter-driver-esp32/README.mdpython/packages/jumpstarter-driver-esp32/examples/exporter.yamlpython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/__init__.pypython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/client.pypython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.pypython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver_test.pypython/packages/jumpstarter-driver-esp32/pyproject.toml
Comment on lines
+36
to
+44
| def _connect_esp(self): | ||
| self.logger.debug("Connecting to ESP32...") | ||
| esp = esptool.cmds.detect_chip( | ||
| port=self.port, | ||
| baud=self.baudrate, | ||
| connect_mode="default_reset", | ||
| trace_enabled=False, | ||
| connect_attempts=7, | ||
| ) |
Contributor
There was a problem hiding this comment.
chip is exposed but never used.
Esp32Flasher accepts a chip config value, but _connect_esp() always auto-detects and never reads self.chip, so changing that setting has no effect. Either wire it into the esptool path or remove it from the public config/docs surface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.py`
around lines 36 - 44, The code always auto-detects the ESP32 in _connect_esp()
using esptool.cmds.detect_chip, ignoring the Esp32Flasher configuration value
self.chip; update _connect_esp to honor that config by: if self.chip is set, use
it instead of auto-detect (either pass the chip name into the esptool call if
the API supports a chip/chip_name parameter, or skip detect_chip and
construct/choose the appropriate esp descriptor based on self.chip), otherwise
fall back to esptool.cmds.detect_chip; reference Esp32Flasher, _connect_esp,
self.chip, and esptool.cmds.detect_chip when making the change (or remove the
public self.chip option and its docs if you prefer not to support manual chip
selection).
Basic driver to interact with esp32 devices Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com> Assisted-by: claude-opus-4.6
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
python/packages/jumpstarter-driver-esp32/pyproject.toml (1)
13-13:⚠️ Potential issue | 🟠 MajorFix the Click version constraint.
click>=8.1.7.2does not exist on PyPI—the latest 8.1.x release is8.1.7. This constraint will cause installation failures.🔧 Suggested fix
- "click>=8.1.7.2", + "click>=8.1.7",#!/bin/bash # Verify click versions on PyPI curl -s https://pypi.org/pypi/click/json | jq -r '.releases | keys[]' | grep "^8\.1" | sort -V🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-esp32/pyproject.toml` at line 13, The dependency line "click>=8.1.7.2" is invalid on PyPI; update the constraint to a valid version range such as "click>=8.1.7,<9" (or pin to "click==8.1.7" if you want an exact version) so installations no longer fail; locate and replace the exact token "click>=8.1.7.2" in pyproject.toml with the chosen valid constraint.
🧹 Nitpick comments (2)
python/packages/jumpstarter-driver-esp32/README.md (1)
51-65: Add a language tag to the CLI code fence.The fenced code block on line 51 is missing a language identifier, which triggers a markdownlint warning. Use
textorconsolefor CLI output examples.📝 Suggested fix
-``` +```text jumpstarter ⚡ local ➤ j esp32 Usage: j esp32 [OPTIONS] COMMAND [ARGS]...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-esp32/README.md` around lines 51 - 65, In the README code fence that shows the CLI output beginning with "jumpstarter ⚡ local ➤ j esp32" (the fenced block containing the Usage/Options/Commands lines), add a language identifier such as text or console to the opening triple backticks (e.g., change ``` to ```text) so the fenced code block is properly tagged and the markdownlint warning is resolved.python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.py (1)
47-52: Consider logging exceptions during cleanup.The bare
except Exception: passsilently discards all errors. While acceptable for cleanup, logging at debug level would aid troubleshooting connection issues.♻️ Proposed improvement
def _close_esp(self, esp): try: if hasattr(esp, "_port") and esp._port: esp._port.close() - except Exception: - pass + except Exception as e: + self.logger.debug("Error closing ESP connection: %s", e)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.py` around lines 47 - 52, In _close_esp, avoid swallowing errors silently: catch Exception as e around the esp._port.close() call and log the exception at debug level (e.g., using the module/class logger) instead of pass; keep the existing hasattr(esp, "_port") and esp._port checks and ensure the log message includes context such as the esp object and the exception to aid troubleshooting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@python/packages/jumpstarter-driver-esp32/pyproject.toml`:
- Line 13: The dependency line "click>=8.1.7.2" is invalid on PyPI; update the
constraint to a valid version range such as "click>=8.1.7,<9" (or pin to
"click==8.1.7" if you want an exact version) so installations no longer fail;
locate and replace the exact token "click>=8.1.7.2" in pyproject.toml with the
chosen valid constraint.
---
Nitpick comments:
In `@python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.py`:
- Around line 47-52: In _close_esp, avoid swallowing errors silently: catch
Exception as e around the esp._port.close() call and log the exception at debug
level (e.g., using the module/class logger) instead of pass; keep the existing
hasattr(esp, "_port") and esp._port checks and ensure the log message includes
context such as the esp object and the exception to aid troubleshooting.
In `@python/packages/jumpstarter-driver-esp32/README.md`:
- Around line 51-65: In the README code fence that shows the CLI output
beginning with "jumpstarter ⚡ local ➤ j esp32" (the fenced block containing the
Usage/Options/Commands lines), add a language identifier such as text or console
to the opening triple backticks (e.g., change ``` to ```text) so the fenced code
block is properly tagged and the markdownlint warning is resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc4940ee-bdcb-41fd-ab58-99ec8f52ab05
⛔ Files ignored due to path filters (1)
python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
python/docs/source/reference/package-apis/drivers/esp32.mdpython/docs/source/reference/package-apis/drivers/index.mdpython/packages/jumpstarter-driver-esp32/.gitignorepython/packages/jumpstarter-driver-esp32/README.mdpython/packages/jumpstarter-driver-esp32/examples/exporter.yamlpython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/__init__.pypython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/client.pypython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.pypython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver_test.pypython/packages/jumpstarter-driver-esp32/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/client.py
- python/docs/source/reference/package-apis/drivers/index.md
- python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver_test.py
this is useful for the esp32 driver, and possibly other drivers in the future Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
It will now use `j serial` and `j storage` to align with other drivers Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com> Assisted-by: claude-opus-4.5
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
python/packages/jumpstarter-driver-pyserial/jumpstarter_driver_pyserial/driver.py (1)
131-148: Repeated connection open/close may cause timing issues for rapid signal toggling.Each call to
set_dtrorset_rtsopens a fresh serial connection and closes it immediately. When the ESP32 driver'senter_bootloader()calls these 5 times in sequence with shorttime.sleep()intervals, each iteration incurs connection overhead and risks timing drift on slower systems.Consider exposing a batched method (e.g.,
set_control_lines(dtr: bool | None, rts: bool | None)) or an optional context-manager variant to hold the port open across multiple toggles. This is not a correctness bug, but may affect reliability on edge hardware.♻️ Optional: Combined control-line method
+ `@export` + def set_control_lines(self, dtr: bool | None = None, rts: bool | None = None): + """Set DTR and/or RTS control signals in a single connection.""" + s = serial_for_url(self.url, baudrate=self.baudrate) + try: + if dtr is not None: + s.dtr = dtr + if rts is not None: + s.rts = rts + finally: + s.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-pyserial/jumpstarter_driver_pyserial/driver.py` around lines 131 - 148, The current set_dtr and set_rts each open/close the serial port causing timing overhead; add a batched method set_control_lines(dtr: bool | None, rts: bool | None) that opens serial_for_url(self.url, baudrate=self.baudrate) once, sets s.dtr and/or s.rts only when the corresponding argument is not None, and then closes the port; additionally expose a context-manager helper (e.g., open_port() or a PortContext class) that yields the open serial instance so callers (like enter_bootloader) can hold the port open across multiple toggles to avoid repeated open/close cycles.python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.py (1)
140-146: Consider validating region bounds.
_parse_region()doesn't validate thatsize > 0or that the region fits within typical flash bounds. While esptool may catch invalid ranges, early validation with a clear error message would improve debuggability.♻️ Optional: Add basic validation
def _parse_region(partition: str | None) -> tuple[int, int]: if partition is None: return (0x0, 0x400000) if ":" in partition: addr_str, size_str = partition.split(":", 1) - return (int(addr_str, 0), int(size_str, 0)) - return (int(partition, 0), 0x400000) + addr, size = int(addr_str, 0), int(size_str, 0) + else: + addr, size = int(partition, 0), 0x400000 + if size <= 0: + raise ValueError(f"Invalid region size: {size}") + return (addr, size)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.py` around lines 140 - 146, The _parse_region function should validate parsed bounds: after computing start and size (for partition None, "addr:size", or single addr), ensure size > 0 and that start >= 0 and start + size does not exceed a defined flash max (introduce or reuse a constant like FLASH_MAX = 0x400000 or similar); on failure raise a clear ValueError describing the invalid region (include parsed start/size and allowed bounds). Update _parse_region to perform these checks for all code paths (partition None, colon form, and single-number form) and return the same tuple only if validation passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.py`:
- Around line 140-146: The _parse_region function should validate parsed bounds:
after computing start and size (for partition None, "addr:size", or single
addr), ensure size > 0 and that start >= 0 and start + size does not exceed a
defined flash max (introduce or reuse a constant like FLASH_MAX = 0x400000 or
similar); on failure raise a clear ValueError describing the invalid region
(include parsed start/size and allowed bounds). Update _parse_region to perform
these checks for all code paths (partition None, colon form, and single-number
form) and return the same tuple only if validation passes.
In
`@python/packages/jumpstarter-driver-pyserial/jumpstarter_driver_pyserial/driver.py`:
- Around line 131-148: The current set_dtr and set_rts each open/close the
serial port causing timing overhead; add a batched method set_control_lines(dtr:
bool | None, rts: bool | None) that opens serial_for_url(self.url,
baudrate=self.baudrate) once, sets s.dtr and/or s.rts only when the
corresponding argument is not None, and then closes the port; additionally
expose a context-manager helper (e.g., open_port() or a PortContext class) that
yields the open serial instance so callers (like enter_bootloader) can hold the
port open across multiple toggles to avoid repeated open/close cycles.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 546723ec-eaca-4dee-82ec-153a6748bfc6
⛔ Files ignored due to path filters (1)
python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
python/packages/jumpstarter-driver-esp32/README.mdpython/packages/jumpstarter-driver-esp32/examples/exporter.yamlpython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver.pypython/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver_test.pypython/packages/jumpstarter-driver-esp32/pyproject.tomlpython/packages/jumpstarter-driver-pyserial/jumpstarter_driver_pyserial/driver.py
🚧 Files skipped from review as they are similar to previous changes (3)
- python/packages/jumpstarter-driver-esp32/examples/exporter.yaml
- python/packages/jumpstarter-driver-esp32/jumpstarter_driver_esp32/driver_test.py
- python/packages/jumpstarter-driver-esp32/README.md
3 tasks
mangelajo
approved these changes
Mar 12, 2026
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
2 participants
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.
Basic driver to interact with esp32 devices
Summary by CodeRabbit
New Features
Documentation
Tests
Chores