Skip to content

Stream memory snapshots to one client IP instead of network broadcast - #369

Open
mullinmax wants to merge 7 commits into
mainfrom
claude/unicast-memory-stream
Open

Stream memory snapshots to one client IP instead of network broadcast#369
mullinmax wants to merge 7 commits into
mainfrom
claude/unicast-memory-stream

Conversation

@mullinmax

Copy link
Copy Markdown
Contributor

Description

/api/memory/toggle-broadcast no longer broadcasts memory snapshots to 255.255.255.255 — it unicasts them to a single target IP. The target is the optional "ip" field in the request body, defaulting to the requesting client's IP, which phew now captures on Request.client_ip from the connection's peername (None over USB, where "ip" must be supplied; a missing/invalid target returns 400).

The target IP is baked into a per-enable closure (_make_memory_snapshot_sender(ip)) that lives only in the scheduler's task list — no module-level state remembers it, so no memory is held at all when streaming is off. A new unschedule_by_name() helper in phew/server.py removes the closure without needing a reference to it, and each enable replaces any previous sender so there is at most one stream target at a time.

The "Broadcast Memory Snapshots" toggle is removed from the admin page entirely (HTML fieldset + initMemoryBroadcastToggle() in admin.js) — this is API-only now, driven by tools like Memory Mapper. Route docs regenerated with tools/gen_api_docs.py.

Related Issues

Companion PRs: python-library#13 (library wrapper gains the ip parameter) and memory-mapper#31 (consumes the new behavior).

Motivation and Context

Broadcasting the full SRAM as UDP to the whole subnet sprayed every Vector board (and everything else) on the network with traffic and caused issues on multi-board networks. Sending directly to the one client that asked eliminates that entirely.

Testing

  • python -m py_compile on the modified Python files; node --check on admin.js.
  • Verified the closure/unschedule_by_name pairing against the scheduler's tuple-based task list (t[0].__name__ matching), and that repeated enables replace rather than accumulate senders.
  • End-to-end tested from the memory-mapper side with a stubbed transport: enable body {"enable": true, "frequency_ms": N, "ip": "..."} and disable {"enable": false}.
  • Not run on hardware — needs a board flash to confirm writer.get_extra_info("peername") and closure __name__ behavior on the deployed MicroPython build.

Screenshots (if applicable)

N/A — the only UI change is the removal of the Debug-section toggle.

Types of Changes

  • Bug fix (non-breaking change to resolve an issue)
  • New feature (non-breaking change to add functionality)
  • Breaking change (alters existing functionality)
  • Documentation update required

Checklist

  • My code follows the project’s style guidelines.
  • I have updated documentation as needed.
  • I have read the CONTRIBUTING.md document.
  • I have added or updated tests.
  • All new and existing tests pass.

Additional Notes

Breaking for callers that enabled the broadcast over USB without an ip (there is no client IP to default to). Packet format on the wire is unchanged (4-byte big-endian offset header + up to 256 data bytes, UDP port 2040), so existing listeners keep working — they just have to be the requested target.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xy1GfVJXuAkgZTMwUDDRTb


Generated by Claude Code

claude and others added 2 commits July 16, 2026 18:04
Broadcasting memory snapshots to 255.255.255.255 sprayed every Vector
board (and everything else) on the network with UDP traffic. The
/api/memory/toggle-broadcast route now unicasts to a single target IP:
the "ip" field in the request body, or by default the requesting
client's IP (now captured on Request.client_ip from the connection's
peername; None over USB, where "ip" must be supplied).

The target IP is baked into a per-enable closure that lives only in the
scheduler's task list — no module-level state remembers it, so nothing
is held in memory when streaming is off. A new unschedule_by_name()
helper in phew removes the closure without needing a reference to it,
and each enable replaces any previous sender so there is always at most
one stream target.

The admin page toggle is removed entirely (HTML + JS); this is API-only
now, driven by tools like Memory Mapper. Route docs regenerated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy1GfVJXuAkgZTMwUDDRTb
@github-actions

Copy link
Copy Markdown
Contributor

Developer build links:
Sys11

https://raw.githubusercontent.com/warped-pinball/vector/pr-update-artifacts/pr-artifacts/pr-369/sys11-update.json

Sys11 (Tiny)

https://raw.githubusercontent.com/warped-pinball/vector/pr-update-artifacts/pr-artifacts/pr-369/sys11-tiny-update.json

WPC

https://raw.githubusercontent.com/warped-pinball/vector/pr-update-artifacts/pr-artifacts/pr-369/wpc-update.json

EM

https://raw.githubusercontent.com/warped-pinball/vector/pr-update-artifacts/pr-artifacts/pr-369/em-update.json

WhiteStar

https://raw.githubusercontent.com/warped-pinball/vector/pr-update-artifacts/pr-artifacts/pr-369/whitestar-update.json

DataEast

https://raw.githubusercontent.com/warped-pinball/vector/pr-update-artifacts/pr-artifacts/pr-369/data-east-update.json

Classic

https://raw.githubusercontent.com/warped-pinball/vector/pr-update-artifacts/pr-artifacts/pr-369/classic-update.json

Comment thread src/common/backend.py
Comment on lines 1771 to 1803
@@ -1742,12 +1788,18 @@ def app_memory_broadcast(request):
elif freq > 60000:
freq = 60000

# Ensure we don't accumulate multiple scheduled entries for the same function
unschedule(broadcast_memory_snapshot)
schedule(broadcast_memory_snapshot, phase_ms=0, frequency_ms=freq) # broadcast every freq milliseconds
# Stream to the explicitly requested IP, or back to whoever asked.
# (Over USB there is no client IP, so "ip" must be supplied.)
ip = data.get("ip") or request.client_ip
if not ip or not _valid_ipv4(str(ip)):
return '{"error":"a target ip is required"}', 400

# Never accumulate senders: one stream target at a time.
unschedule_by_name("send_memory_snapshot")
schedule(_make_memory_snapshot_sender(str(ip)), phase_ms=0, frequency_ms=freq)
else:
# remove function call from scheduler
unschedule(broadcast_memory_snapshot)
# Remove the sender from the scheduler (drops its baked-in IP too)
unschedule_by_name("send_memory_snapshot")
return

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This function can be cleaner by making the disable path a guard clause

@mullinmax
mullinmax marked this pull request as ready for review July 16, 2026 18:44
Copilot AI review requested due to automatic review settings July 16, 2026 18:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 changes the memory snapshot streaming behavior so /api/memory/toggle-broadcast no longer UDP-broadcasts to the subnet, but instead unicasts snapshots to a single target IPv4 address (explicit ip in the request body, or defaulting to the requesting client’s IP captured by phew). It also removes the admin UI toggle for this feature and updates the generated route docs accordingly.

Changes:

  • Capture the requesting client IP in phew.server.Request.client_ip and add unschedule_by_name() for unscheduling closure-based tasks.
  • Update /api/memory/toggle-broadcast to stream snapshots to a single target IP (with optional frequency_ms), replacing any prior sender.
  • Remove the admin page “Broadcast Memory Snapshots” UI and regenerate route docs; bump VectorVersion.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/common/web/js/admin.js Removes the admin JS handler for the memory broadcast toggle.
src/common/web/html/admin.html Removes the “Broadcast Memory Snapshots” toggle UI from the Debug section.
src/common/SharedState.py Bumps VectorVersion to 1.11.28.
src/common/phew/server.py Adds Request.client_ip capture and unschedule_by_name() for scheduled tasks.
src/common/backend.py Switches memory snapshot streaming from broadcast to per-client unicast with IP validation and task replacement.
docs/routes.md Updates autogenerated route documentation for the new request/response shape and handler line references.

Comment thread src/common/backend.py Outdated
Comment thread src/common/backend.py Outdated
mullinmax and others added 5 commits July 16, 2026 14:50
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Address review feedback: the closure docstring implied zero memory
retention; state precisely that the target IP is held only by the
closure in the scheduler's task list and is released on unschedule.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy1GfVJXuAkgZTMwUDDRTb
The build_and_release run failed in a github-script step with a GitHub
5xx error page, unrelated to the changes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy1GfVJXuAkgZTMwUDDRTb
build_and_release failed twice in a row when the git-data API returned
a GitHub-side 500 (Unicorn page) while publishing PR update artifacts;
the step ran with retries: 0 so a single transient server error failed
the whole build. Let github-script retry these calls (5xx is retried by
default; 4xx stays exempt).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy1GfVJXuAkgZTMwUDDRTb
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.

3 participants