Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR creates a new control backend for the simulator by introducing an asynchronous TCP broadcaster and updating configuration, client, and controller components to support the new backend architecture. Key changes include:
- Adding a new TCP broadcaster implementation with asynchronous client handling
- Converting controller and main application logic to async with Axum integration
- Updating configuration structures and CLI components to align with the new design
Reviewed Changes
Copilot reviewed 45 out of 46 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| antares/src/radar/broadcaster/tcp.rs | Introduces a TCP broadcaster with async client handling using Tokio |
| antares/src/radar/broadcaster/mod.rs | Exposes broadcast modules and re-exports implementations |
| antares/src/radar/broadcaster/broadcaster.rs | Defines the broadcaster trait used by different implementations |
| antares/src/main.rs | Converts the main entry point to async with Axum server integration |
| antares/src/controller.rs | Updates Controller to use async operations and spawns simulation tasks |
| antares/src/config.rs | Adds a Default implementation for configuration |
| antares/assets/config.toml | Updates the configuration layout to match new simulation and radar specs |
| antares/Cargo.toml | Updates package name and dependencies |
| antares-python/* | Adjusts Python client, CLI, and tests to work with the updated config/API |
Files not reviewed (1)
- antares-python/template.env: Language not supported
Comments suppressed due to low confidence (1)
antares/src/controller.rs:29
- [nitpick] Consider capturing and handling the join handle from tokio::spawn to monitor task failures where appropriate.
tokio::spawn(async move { simulation.start(wave_sender).await; });
| while let Ok(track) = receiver.recv().await { | ||
| let csv_line = format!("{}\n", track.serialize()); | ||
| if let Err(e) = stream.write_all(csv_line.as_bytes()).await { | ||
| eprintln!("Failed to send data to TCP client: {}", e); |
There was a problem hiding this comment.
[nitpick] Consider using a structured logger instead of eprintln for consistent logging across the application.
| controller_bind_addr=data.get("antares.simulation.controller_bind_addr", "0.0.0.0:17394"), | ||
| radar_bind_addr=data.get("antares.simulation.radar_bind_addr", "0.0.0.0:17396"), |
There was a problem hiding this comment.
[nitpick] Ensure that the configuration keys used here (e.g. 'antares.simulation.controller_bind_addr' and 'antares.simulation.radar_bind_addr') are consistent with the updated TOML structure.
| controller_bind_addr=data.get("antares.simulation.controller_bind_addr", "0.0.0.0:17394"), | |
| radar_bind_addr=data.get("antares.simulation.radar_bind_addr", "0.0.0.0:17396"), | |
| controller_bind_addr=data.get("antares.network.controller_bind_addr", "0.0.0.0:17394"), | |
| radar_bind_addr=data.get("antares.network.radar_bind_addr", "0.0.0.0:17396"), |
There was a problem hiding this comment.
Pull Request Overview
This PR implements the control backend for the simulator by updating configuration structures, client connection parameters, CLI behaviors, and related documentation.
- Updated ESLint configuration and documentation for the web interface
- Refactored configuration handling and client initialization in the Python backend
- Revised CI workflows and dependency definitions
Reviewed Changes
Copilot reviewed 180 out of 180 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| antares-web/eslint.config.js | Added new ESLint configuration using TS and React plugins |
| antares-web/config.example.toml | Updated simulation and radar configuration settings |
| antares-web/components.json | Introduced component path aliases |
| antares-web/README.md | Updated documentation to reflect new settings and units |
| antares-python/tests/test_cli.py | Modified tests to work with updated CLI and configuration |
| antares-python/template.env | Removed unused legacy variables and added new addresses |
| antares-python/src/antares/config_loader.py | Changed configuration key mapping to use simulation keys |
| antares-python/src/antares/config.py | Dropped deprecated parameters and fields |
| antares-python/src/antares/client/rest.py | Removed support for auth token as part of API client update |
| antares-python/src/antares/client/init.py | Updated client initialization to use new configuration keys |
| antares-python/src/antares/cli.py | Updated CLI output and configuration building logic |
| antares-python/pyproject.toml | Expanded dev dependencies and tooling |
| antares-python/main.py | Updated client instantiation and unit display |
| antares-python/config.example.toml | Revised configuration structure for simulation and radar |
| antares-python/README.md | Updated instructions to reflect configuration changes |
| README.md | Adjusted overall project documentation and structure display |
| .github/workflows/rust-release.yml | New workflow to create Rust releases with placeholder in 'bin' field |
| .github/workflows/deploy-docs.yml | Workflow to deploy documentation to GitHub Pages |
| with: | ||
| # (required) Comma-separated list of binary names (non-extension portion of filename) to build and upload. | ||
| # Note that glob pattern is not supported yet. | ||
| bin: ... |
There was a problem hiding this comment.
Replace the placeholder '...' in the bin field with the actual binary names to ensure the release workflow properly builds and uploads the intended artifacts.
| bin: ... | |
| bin: myapp,mytool |
| return AntaresSettings(**data.get("antares", {})) | ||
| return AntaresSettings( | ||
| controller_bind_addr=data.get("antares.simulation.controller_bind_addr", "0.0.0.0:17394"), | ||
| radar_bind_addr=data.get("antares.simulation.radar_bind_addr", "0.0.0.0:17396"), |
There was a problem hiding this comment.
The configuration loader is expecting the key 'antares.simulation.radar_bind_addr', but the example config file uses 'bind_addr' under the [antares.radar] section. Consider aligning these keys to avoid misconfiguration.
| radar_bind_addr=data.get("antares.simulation.radar_bind_addr", "0.0.0.0:17396"), | |
| radar_bind_addr=data.get("antares.radar", {}).get("bind_addr", "0.0.0.0:17396"), |
1 participant
No description provided.