Multitraffic conn#8
Conversation
Signed-off-by: Sidartha-CT <[email protected]>
Signed-off-by: Sidartha-CT <[email protected]>
Signed-off-by: Sidartha-CT <[email protected]>
…rall_report DIR. Verified CLI : python3 lf_multi_traffic.py --mgr 192.168.207.75 --upstream_port eth1 --series_tests zoom_test --zoom_signin_email [email protected] --zoom_signin_passwd 'CANDELAtech1@530048' --zoom_duration 2 --zoom_host 1.56 --zoom_participants 2 --zoom_device_list 1.40,1.56 --zoom_audio --zoom_video --api_stats_collection Signed-off-by: Sidartha-CT <[email protected]>
Verified CLI : python3 lf_multi_traffic.py --config --device_list all --ssid MyNetwork --security wpa2 --passwd MyPassword Signed-off-by: Sidartha-CT <[email protected]>
Signed-off-by: Sidartha-CT <[email protected]>
Signed-off-by: Sidartha-CT <[email protected]>
…rallel (distinct devices) Signed-off-by: Sidartha-CT <[email protected]>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09915f2083
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ai_jit = float(row["audio_input_jitter_avg"] or 0) | ||
| ao_jit = float(row["audio_output_jitter_avg"] or 0) | ||
| ai_lat = float(row["audio_input_latency_avg"] or 0) | ||
| ao_lat = float(row["audio_output_latency_avg"] or 0) | ||
| ai_loss = float(row["audio_input_avg_loss_avg"] or 0) |
There was a problem hiding this comment.
When --api_stats_collection is used, the API CSV columns are reversed here: lf_interop_zoom.py maps audio_output_*/video_output_* to Sending and audio_input_*/video_input_* to Receiving, but this branch assigns input metrics to values later appended to the _s (sent) series and output metrics to _r (received). This makes the overall Zoom report graphs and min/max tables swap sent vs received data for API-based reports.
Useful? React with 👍 / 👎.
| exit(0) | ||
| # Real-app tests are allowed in parallel only when every test targets a distinct set of devices. | ||
| # zoom_host is included in zoom_test's device set because it participates in the session. | ||
| real_app_parallel = [t for t in tests_to_run_parallel if t in ("rb_test", "teams_test", "yt_test", "zoom_test")] |
There was a problem hiding this comment.
This now allows rb_test, yt_test, and zoom_test in parallel for every context, but in robot/band-steering flows the network tests call wait_for_all_tests_to_finish(), which waits on yt_done_event, rb_done_event, and zoom_done_event whenever those names are in self.parallel_tests; those events are only declared/waited and no real-app runner sets them (checked repo-wide), so a mixed run such as --robot_test --parallel_tests ping_test,yt_test hangs once ping reaches the synchronization point. Please either keep these real-apps out of robot parallel runs or signal the events from their runners.
Useful? React with 👍 / 👎.
| parser.add_argument('--security', default='open', help='WiFi Security protocol: < open | wep | wpa | wpa2 | wpa3 >') | ||
| parser.add_argument('--file_name', type=str, default='', help='DeviceConfig group file name') | ||
| parser.add_argument('--wait_time', type=int, default=60, help='Max seconds to wait for device configuration') | ||
| parser.add_argument('--app_flags', default=None, help='App flags forwarded to DeviceConfig') |
There was a problem hiding this comment.
With the default CLI value here, --config passes None into DeviceConfig; for Android/ADB devices, ADB_DEVICES.configure_wifi() later executes self.app_flags.lower() before posting the Wi-Fi command, so a normal invocation like --config --device_list <android> --ssid ... --security wpa2 --passwd ... crashes unless the user also supplies --app_flags. Please default this to the string value DeviceConfig already expects or coerce None before constructing it.
Useful? React with 👍 / 👎.
| "zoom_test": getattr(args, "zoom_device_list", None) or "", | ||
| "teams_test": getattr(args, "teams_device_list", None) or "", | ||
| } | ||
| devices = set(d.strip() for d in raw_map[test_name].split(',') if d.strip()) |
There was a problem hiding this comment.
If a parallel real-app test omits its device-list argument, this defaults the list to "" and returns an empty set, so combinations like --parallel_tests rb_test,yt_test pass the non-overlap check even though the runners will prompt/select devices later and may target the same clients concurrently. For the new parallel real-app mode, missing device lists should be rejected (or resolved before this check) so the distinct-device guarantee is actually enforceable.
Useful? React with 👍 / 👎.
| duration_to_skip=args.duration_to_skip, | ||
| test_map=test_map) | ||
| if args.config: | ||
| multi_traffic_obj.configure_devices( |
There was a problem hiding this comment.
When --config is combined with test execution, configure_devices() can return an empty list for common failures such as missing --ssid, invalid password/security combinations, or no valid devices after filtering, but this call ignores the result and proceeds to run the requested tests. That produces traffic results against stale or unconfigured Wi-Fi clients; please stop before running tests when configuration was requested and no devices were successfully configured.
Useful? React with 👍 / 👎.
1 participant
No description provided.