Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR appears to streamline/unify the client implementations by removing multiple legacy ESP32 client projects (HTTP/MQTT/WebSocket variants and their embedded per-layer model headers), and adding a local simulation script plus model metadata JSON files to support testing.
Changes:
- Removed PlatformIO-based ESP32 client projects’ configuration/docs and large embedded
layer_*.hmodel blobs (WebSocket + MQTT + HTTP variants). - Added a Python simulation harness to run server/client scenarios (no server, crash/recovery, parallel clients).
- Updated Python dependencies and added JSON model metadata (layer sizes + valid offloading points).
Reviewed changes
Copilot reviewed 121 out of 1160 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_32.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_29.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_28.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_25.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_22.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_2.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_19.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_14.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_11.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/model_layers/layer_10.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-websocket/src/conf.template.h | Removed ESP32 WebSocket client configuration template |
| src/client/esp32-s3-eye-websocket/platformio.ini | Removed ESP32 WebSocket client PlatformIO config |
| src/client/esp32-s3-eye-websocket/lib/README | Removed PlatformIO lib directory README |
| src/client/esp32-s3-eye-websocket/include/README | Removed PlatformIO include directory README |
| src/client/esp32-s3-eye-websocket/README.md | Removed ESP32 WebSocket client README |
| src/client/esp32-s3-eye-websocket/.gitignore | Removed ESP32 WebSocket client gitignore |
| src/client/esp32-s3-eye-mqtt/test/README | Removed PlatformIO test directory README |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layers.h | Removed layer include/dispatch header |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_58.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_55.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_52.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_5.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_49.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_46.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_43.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_40.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_37.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_32.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_29.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_28.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_25.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_22.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_2.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_19.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_14.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/model_layers/layer_10.h | Removed embedded model layer header blob |
| src/client/esp32-s3-eye-mqtt/src/conf.template.h | Removed ESP32 MQTT client configuration template |
| src/client/esp32-s3-eye-mqtt/platformio.ini | Removed ESP32 MQTT client PlatformIO config |
| src/client/esp32-s3-eye-mqtt/lib/README | Removed PlatformIO lib directory README |
| src/client/esp32-s3-eye-mqtt/include/README | Removed PlatformIO include directory README |
| src/client/esp32-s3-eye-mqtt/README.md | Removed ESP32 MQTT client README |
| src/client/esp32-s3-eye-mqtt/.gitignore | Removed ESP32 MQTT client gitignore |
| src/client/esp32-s3-eye-http/test/README | Removed PlatformIO test directory README |
| src/client/esp32-s3-eye-http/src/conf.template.h | Removed ESP32 HTTP client configuration template |
| src/client/esp32-s3-eye-http/platformio.ini | Removed ESP32 HTTP client PlatformIO config |
| src/client/esp32-s3-eye-http/lib/README | Removed PlatformIO lib directory README |
| src/client/esp32-s3-eye-http/include/README | Removed PlatformIO include directory README |
| src/client/esp32-s3-eye-http/README.md | Removed ESP32 HTTP client README |
| src/client/esp32-s3-eye-http/.gitignore | Removed ESP32 HTTP client gitignore |
| src/client/README.md | Removed top-level clients directory README |
| scripts/simulation/test_file.py | Added Python simulation script for server/client scenarios |
| pyproject.toml | Updated dependency list (removed Flask deps, changed h5py constraint) |
| data/models/valid_offloading_points_test_model_144x144.json | Added valid offloading points data |
| data/models/layer_sizes_test_model_96x96.json | Added layer-size metadata for 96x96 test model |
| data/models/layer_sizes_test_model_144x144.json | Added layer-size metadata for 144x144 test model |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import subprocess | ||
| import time | ||
| import sys | ||
| import os |
There was a problem hiding this comment.
os is imported but not used anywhere in this file. Removing unused imports helps keep the script clean and avoids linter failures (e.g., Ruff/F401).
| import os |
| # Faccio eseguire solo il test parallelo per risparmiare tempo | ||
| test_client_without_server() | ||
| time.sleep(1) | ||
| test_server_crash_and_recovery() | ||
| time.sleep(1) | ||
|
|
||
| test_parallel_clients() |
There was a problem hiding this comment.
The comment says “only the parallel test” will run, but the code runs all three tests. Please either update the comment or adjust the executed tests so the comment matches actual behavior.
| server_process.terminate() | ||
| server_process.wait() |
There was a problem hiding this comment.
terminate() + wait() without a timeout can hang indefinitely if the child process ignores SIGTERM or is stuck (common with servers). Consider adding a timeout to wait(), and if it expires, call kill() (and/or terminate the whole process group) so the simulation suite doesn’t stall and leave orphaned processes behind.
| client_process.terminate() | ||
| server_process2.terminate() | ||
| client_process.wait() | ||
| server_process2.wait() |
There was a problem hiding this comment.
terminate() + wait() without a timeout can hang indefinitely if the child process ignores SIGTERM or is stuck (common with servers). Consider adding a timeout to wait(), and if it expires, call kill() (and/or terminate the whole process group) so the simulation suite doesn’t stall and leave orphaned processes behind.
|
|
||
| print("[TEST] Avvio il Server...") | ||
| server_process = subprocess.Popen(SERVER_CMD, cwd=PROJECT_ROOT) | ||
| time.sleep(5) |
There was a problem hiding this comment.
The script relies on many hard-coded sleep durations. To make the suite easier to tune across environments (fast/slow machines, CI), define these as named constants at the top of the file or accept them via CLI args (e.g., argparse) so test timing is configurable without editing the code.
| print("[TEST] Avvio il Client. Dovrebbero iniziare a comunicare...") | ||
| client_process = subprocess.Popen(CLIENT_CMD, cwd=PROJECT_ROOT) | ||
|
|
||
| time.sleep(10) |
There was a problem hiding this comment.
The script relies on many hard-coded sleep durations. To make the suite easier to tune across environments (fast/slow machines, CI), define these as named constants at the top of the file or accept them via CLI args (e.g., argparse) so test timing is configurable without editing the code.
| server_process.wait() | ||
|
|
||
| print("[TEST] Il server è morto. Osserviamo come reagisce il client per 10 secondi...") | ||
| time.sleep(10) |
There was a problem hiding this comment.
The script relies on many hard-coded sleep durations. To make the suite easier to tune across environments (fast/slow machines, CI), define these as named constants at the top of the file or accept them via CLI args (e.g., argparse) so test timing is configurable without editing the code.
| server_process2 = subprocess.Popen(SERVER_CMD, cwd=PROJECT_ROOT) | ||
|
|
||
| print("[TEST] Vediamo se il client riesce a riconnettersi e riprendere l'offloading...") | ||
| time.sleep(15) |
There was a problem hiding this comment.
The script relies on many hard-coded sleep durations. To make the suite easier to tune across environments (fast/slow machines, CI), define these as named constants at the top of the file or accept them via CLI args (e.g., argparse) so test timing is configurable without editing the code.
| time.sleep(0.5) | ||
|
|
||
| print(f"\n[TEST] Lascio lavorare tutti i client per 10 secondi...") | ||
| time.sleep(10) # Ho alzato a 10s per dargli un po' di respiro |
There was a problem hiding this comment.
The script relies on many hard-coded sleep durations. To make the suite easier to tune across environments (fast/slow machines, CI), define these as named constants at the top of the file or accept them via CLI args (e.g., argparse) so test timing is configurable without editing the code.
| @@ -0,0 +1,116 @@ | |||
| import subprocess | |||
There was a problem hiding this comment.
The filename test_file.py is too generic for a growing scripts/simulation directory. Renaming it to something specific like simulation_suite.py or client_server_simulation.py will make it easier to discover and reference.
No description provided.