Skip to content

Create control backend for simulator #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
May 9, 2025

Conversation

jsurrea
Copy link
Collaborator

@jsurrea jsurrea commented May 7, 2025

No description provided.

@jsurrea jsurrea requested a review from Copilot May 7, 2025 05:26
@jsurrea jsurrea self-assigned this May 7, 2025
@jsurrea jsurrea added enhancement New feature or request feature labels May 7, 2025
Copy link

@Copilot 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 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);
Copy link
Preview

Copilot AI May 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using a structured logger instead of eprintln for consistent logging across the application.

Copilot uses AI. Check for mistakes.

Comment on lines +21 to +22
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"),
Copy link
Preview

Copilot AI May 7, 2025

Choose a reason for hiding this comment

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

[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.

Suggested change
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"),

Copilot uses AI. Check for mistakes.

@jsurrea jsurrea requested a review from Copilot May 9, 2025 00:21
@jsurrea jsurrea merged commit f363b25 into main May 9, 2025
2 checks passed
Copy link

@Copilot 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 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: ...
Copy link
Preview

Copilot AI May 9, 2025

Choose a reason for hiding this comment

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

Replace the placeholder '...' in the bin field with the actual binary names to ensure the release workflow properly builds and uploads the intended artifacts.

Suggested change
bin: ...
bin: myapp,mytool

Copilot uses AI. Check for mistakes.

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"),
Copy link
Preview

Copilot AI May 9, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
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"),

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant