Skip to content

Feature/serial buffer options#95

Merged
JPHutchins merged 3 commits into
mainfrom
feature/serial-buffer-options
Apr 20, 2026
Merged

Feature/serial buffer options#95
JPHutchins merged 3 commits into
mainfrom
feature/serial-buffer-options

Conversation

@JPHutchins
Copy link
Copy Markdown
Collaborator

Long overdue feature to allow setting the "line length" and "line buffers". "mtu" is deprecated, but will continue being interpreted as it always has: line_length = mtu, line_buffers = 1.

Many investigations and conversations have confirmed that the line length never should have been configurable in the first place. Including in Zephyr, MCUboot etc. But, we need to continue supporting non-standard line lengths since there are many units in the field that changed the line length as a speed up.

Also committed is an LLM generated report regarding naming and defaults.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds explicit serial transport framing/buffering controls to the smpmgr CLI while keeping --mtu working for backwards compatibility (now deprecated for serial).

Changes:

  • Introduces --line-length and --line-buffers CLI options and threads them into the runtime Options.
  • Updates serial transport initialization to derive max_smp_encoded_frame_size = line_length * line_buffers, with legacy --mtu mapped to line_length=mtu, line_buffers=1 and guarded against mixed usage.
  • Adds an LLM-generated research note on SMP serial parameter naming/defaults and updates VS Code workspace settings for Poetry env/package management.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
smpmgr/main.py Adds CLI flags for serial line length/buffer count; updates --mtu help text and passes new values into Options.
smpmgr/common.py Adds defaults and implements serial transport argument selection/validation (including --mtu deprecation behavior).
docs/llm/smp-serial-naming-research.md Adds a research document supporting naming/default decisions for SMP serial parameters.
.vscode/settings.json Sets Poetry as the default env/package manager for VS Code’s Python environment tooling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JPHutchins JPHutchins force-pushed the feature/serial-buffer-options branch from cef4e01 to 18ca293 Compare April 14, 2026 18:20
@JPHutchins JPHutchins force-pushed the feature/serial-buffer-options branch from 18ca293 to d590a94 Compare April 14, 2026 18:22
@JPHutchins
Copy link
Copy Markdown
Collaborator Author

JPHutchins commented Apr 20, 2026

Tested with some HW that has line length 4096 and line buffers 1, and it seems good.

 >  poetry run smpmgr --port /dev/ttyACM0 os echo hello
[10:44:35] WARNING  Error reading MCUMgr parameters: header=Header(op=<OP.READ_RSP:  __init__.py:534
                    1>, version=<Version.V2: 1>, flags=<Flag.UNUSED: 0>, length=6,
                    group_id=0, sequence=0, command_id=6) version=<Version.V2: 1>
                    sequence=0
                    smp_data=b'\t\x00\x00\x06\x00\x00\x00\x06\xbfbrc\x08\xff'
                    rc=<MGMT_ERR.ENOTSUP: 8> rsn=None - __init__:534
⠋ Connecting to /dev/ttyACM0... OK
⠋ Waiting for response to EchoWrite... OK
EchoWriteResponse(
    header=Header(
        op=<OP.WRITE_RSP: 3>,
        version=<Version.V2: 1>,
        flags=<Flag.UNUSED: 0>,
        length=10,
        group_id=0,
        sequence=1,
        command_id=0
    ),
    version=<Version.V2: 1>,
    sequence=1,
    smp_data=b'\x0b\x00\x00\n\x00\x00\x01\x00\xbfarehello\xff',
    r='hello'
)
   ~/repos/smpmgr   feature/serial-buffer-opt ≡  1  1.914s   
 >  poetry run smpmgr --port=/dev/ttyACM0 --mtu=4096 os echo hello
Warning: --mtu is deprecated for serial transport. Use --line-length and --line-buffers instead. --mtu 4096 has been applied as --line-length 4096 --line-buffers 1.
[10:44:52] WARNING  Error reading MCUMgr parameters: header=Header(op=<OP.READ_RSP:  __init__.py:534
                    1>, version=<Version.V2: 1>, flags=<Flag.UNUSED: 0>, length=6,
                    group_id=0, sequence=0, command_id=6) version=<Version.V2: 1>
                    sequence=0
                    smp_data=b'\t\x00\x00\x06\x00\x00\x00\x06\xbfbrc\x08\xff'
                    rc=<MGMT_ERR.ENOTSUP: 8> rsn=None - __init__:534
⠋ Connecting to /dev/ttyACM0... OK
⠋ Waiting for response to EchoWrite... OK
EchoWriteResponse(
    header=Header(
        op=<OP.WRITE_RSP: 3>,
        version=<Version.V2: 1>,
        flags=<Flag.UNUSED: 0>,
        length=10,
        group_id=0,
        sequence=1,
        command_id=0
    ),
    version=<Version.V2: 1>,
    sequence=1,
    smp_data=b'\x0b\x00\x00\n\x00\x00\x01\x00\xbfarehello\xff',
    r='hello'
)
   ~/repos/smpmgr   feature/serial-buffer-opt ≡  1  1.954s   
 >  poetry run smpmgr --port=/dev/ttyACM0 --line-length=4096 os echo hello
[10:45:13] WARNING  Error reading MCUMgr parameters: header=Header(op=<OP.READ_RSP:  __init__.py:534
                    1>, version=<Version.V2: 1>, flags=<Flag.UNUSED: 0>, length=6,
                    group_id=0, sequence=0, command_id=6) version=<Version.V2: 1>
                    sequence=0
                    smp_data=b'\t\x00\x00\x06\x00\x00\x00\x06\xbfbrc\x08\xff'
                    rc=<MGMT_ERR.ENOTSUP: 8> rsn=None - __init__:534
⠋ Connecting to /dev/ttyACM0... OK
⠋ Waiting for response to EchoWrite... OK
EchoWriteResponse(
    header=Header(
        op=<OP.WRITE_RSP: 3>,
        version=<Version.V2: 1>,
        flags=<Flag.UNUSED: 0>,
        length=10,
        group_id=0,
        sequence=1,
        command_id=0
    ),
    version=<Version.V2: 1>,
    sequence=1,
    smp_data=b'\x0b\x00\x00\n\x00\x00\x01\x00\xbfarehello\xff',
    r='hello'
)
   ~/repos/smpmgr   feature/serial-buffer-opt ≡  1  1.83s   
 >  poetry run smpmgr --port=/dev/ttyACM0 --line-length=4096 --line-buffers=1 os echo hello
[10:45:22] WARNING  Error reading MCUMgr parameters: header=Header(op=<OP.READ_RSP:  __init__.py:534
                    1>, version=<Version.V2: 1>, flags=<Flag.UNUSED: 0>, length=6,
                    group_id=0, sequence=0, command_id=6) version=<Version.V2: 1>
                    sequence=0
                    smp_data=b'\t\x00\x00\x06\x00\x00\x00\x06\xbfbrc\x08\xff'
                    rc=<MGMT_ERR.ENOTSUP: 8> rsn=None - __init__:534
⠋ Connecting to /dev/ttyACM0... OK
⠋ Waiting for response to EchoWrite... OK
EchoWriteResponse(
    header=Header(
        op=<OP.WRITE_RSP: 3>,
        version=<Version.V2: 1>,
        flags=<Flag.UNUSED: 0>,
        length=10,
        group_id=0,
        sequence=1,
        command_id=0
    ),
    version=<Version.V2: 1>,
    sequence=1,
    smp_data=b'\x0b\x00\x00\n\x00\x00\x01\x00\xbfarehello\xff',
    r='hello'
)

@JPHutchins JPHutchins merged commit e24da6e into main Apr 20, 2026
13 checks passed
@JPHutchins JPHutchins deleted the feature/serial-buffer-options branch April 20, 2026 17:50
wayyoung added a commit to wayyoung/msmpmgr that referenced this pull request May 23, 2026
Brings upstream main into the msmpmgr fork. Key changes:

- main: --line-length / --line-buffers serial options (PR intercreate#95)
- main: --format=mcuboot|any replaces --bypass-inspect (PR intercreate#94/intercreate#93)
- main: SMPClient timeout refactor (PR intercreate#86) — partially adopted

Conflict resolutions:
- smpmgr/ → msmpmgr/ package rename preserved; smpmgr/common.py,
  smpmgr/enumeration_management.py, smpmgr/shell_management.py, and
  smpmgr/user/intercreate.py removed (ports now live under msmpmgr/).
- Upstream example plugins (plugins/another/, plugins/example_group.py)
  dropped — the fork ships its own plugins.
- msmpmgr/user/intercreate.py ported from smpmgr/user/intercreate.py
  with the import rewritten to msmpmgr.common.
- main.py: combined imports (kept msmpmgr.* paths, picked up
  DEFAULT_LINE_BUFFERS/DEFAULT_LINE_LENGTH, ImageFormat/ImageFormatOption,
  intercreate). Adopted upstream's `match format:` block for image-hash
  resolution in the upgrade() command, with `options` re-added to the
  smp_request() call to match this fork's helper signature.
- common.py: added DEFAULT_LINE_LENGTH=128, DEFAULT_LINE_BUFFERS=2 and
  documented the intentional divergence from upstream commit 688cef6
  (smp_request / smp_request_no_spinner / ping_connect keep their
  `options` parameter for external-plugin compatibility — see comment
  block above smp_request).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants