Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses XML file errors caused by licensing issues in SUMO configuration files. The changes involve adding MIT License headers to key XML files, fixing XML structure issues, and enabling multi-client TraCI connections for the SUMO traffic simulator.
- Adds MIT License headers to Nga4ThuDuc scenario XML files
- Configures SUMO to support multiple TraCI connections via
--num-clients 2parameter - Includes two utility scripts for resolving Git conflicts and fixing XML file structure
- Updates the active scenario from Nga4ThuDuc to QuangTrung
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/backend/app/sumo_rl/sumo_files/current_scenario.txt | Switches active scenario to QuangTrung (contains formatting issue) |
| src/backend/app/sumo_rl/sumo_files/QuangTrung/trips.xml | Adds blank lines before XML declaration (invalid XML structure) |
| src/backend/app/sumo_rl/sumo_files/NguyenThaiSon/*.xml | Updates timestamps and adds num-clients parameter to 17 configuration files |
| src/backend/app/sumo_rl/sumo_files/Nga4ThuDuc/*.xml | Adds MIT License headers, updates timestamps, adds num-clients parameter, and clears detector data |
| src/backend/app/sumo_rl/sumo_files/Nga4ThuDuc/.tmp | Creates temporary XML file (incomplete structure) |
| src/backend/app/sumo_rl/agents/iot_agent.py | Adds --num-clients parameter to SUMO startup command with updated documentation |
| src/backend/Dockerfile | Increases pip install timeout to 600 seconds for reliability |
| resolve_conflicts.py | Utility script for resolving Git conflicts in XML files (contains hardcoded paths) |
| fix_xml_files.py | Utility script for fixing XML declaration positioning (contains hardcoded paths) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return resolved | ||
|
|
||
| def main(): | ||
| nga4_dir = Path('/home/thaianh/Study/GreenWave/src/backend/app/sumo_rl/sumo_files/Nga4ThuDuc') |
There was a problem hiding this comment.
Hardcoded absolute path /home/thaianh/Study/GreenWave/src/backend/app/sumo_rl/sumo_files/Nga4ThuDuc makes this script non-portable. Use a relative path or make the path configurable via command-line arguments. For example:
nga4_dir = Path(__file__).parent / 'src/backend/app/sumo_rl/sumo_files/Nga4ThuDuc'or accept it as a command-line argument.
| @@ -1 +1 @@ | |||
| Nga4ThuDuc No newline at end of file | |||
| QuangTrung No newline at end of file | |||
There was a problem hiding this comment.
The scenario name now includes a line number prefix " 1 |" which will be read as part of the scenario name by read_scenario() in launcher.py (line 29 uses f.read().strip()). This will cause the scenario lookup to fail since the actual directory is named "QuangTrung", not " 1 |QuangTrung". Remove the line number prefix and only keep "QuangTrung".
| conflict_pattern = r'<<<<<<< HEAD\n(.*?)\n=======\n(.*?)\n>>>>>>> .*?\n' | ||
| resolved = re.sub( | ||
| conflict_pattern, | ||
| r'\1\n', | ||
| content, | ||
| flags=re.DOTALL | ||
| ) | ||
|
|
||
| # Second pass: Remove any remaining standalone markers | ||
| resolved = re.sub(r'<<<<<<< HEAD\n?', '', resolved) | ||
| resolved = re.sub(r'=======\n?', '', resolved) | ||
| resolved = re.sub(r'>>>>>>> .*?\n?', '', resolved) |
There was a problem hiding this comment.
The regex pattern doesn't handle all edge cases properly. The pattern requires a newline after >>>>>>> but the marker might appear at the end of the file without a trailing newline. Consider using:
conflict_pattern = r'<<<<<<< HEAD\n(.*?)\n=======\n(.*?)\n>>>>>>> [^\n]*\n?'Also, the second pass removals should use re.MULTILINE flag to properly handle line boundaries.
| #!/usr/bin/env python3 | ||
| """ | ||
| Resolve Git merge conflicts in XML files | ||
| Keeps the HEAD version (current branch) and removes conflict markers | ||
| """ | ||
|
|
There was a problem hiding this comment.
[nitpick] If this is a one-time utility script used to fix the XML files (as the PR description suggests), it should be removed from the repository after being run, or clearly documented in comments or a README that it's a maintenance script not meant for regular use. Consider adding a comment at the top explaining its purpose and when it should be used.
| #!/usr/bin/env python3 | ||
| """Fix XML files to have XML declaration at the beginning""" | ||
|
|
||
| import os | ||
| import re | ||
| from pathlib import Path |
There was a problem hiding this comment.
[nitpick] If this is a one-time utility script used to fix the XML files (as the PR description suggests), it should be removed from the repository after being run, or clearly documented in comments or a README that it's a maintenance script not meant for regular use. Consider adding a comment at the top explaining its purpose and when it should be used.
| #!/usr/bin/env python3 | ||
| """ | ||
| Resolve Git merge conflicts in XML files | ||
| Keeps the HEAD version (current branch) and removes conflict markers | ||
| """ | ||
|
|
||
| import os | ||
| import re | ||
| from pathlib import Path | ||
|
|
||
| def resolve_conflict(content): | ||
| """ | ||
| Remove Git conflict markers and keep HEAD version | ||
|
|
||
| Handles multiple patterns: | ||
| 1. <<<<<<< HEAD ... ======= ... >>>>>>> branch | ||
| 2. Standalone markers that weren't cleaned up | ||
| """ | ||
| # First pass: Remove complete conflict blocks, keep HEAD content | ||
| conflict_pattern = r'<<<<<<< HEAD\n(.*?)\n=======\n(.*?)\n>>>>>>> .*?\n' | ||
| resolved = re.sub( | ||
| conflict_pattern, | ||
| r'\1\n', | ||
| content, | ||
| flags=re.DOTALL | ||
| ) | ||
|
|
||
| # Second pass: Remove any remaining standalone markers | ||
| resolved = re.sub(r'<<<<<<< HEAD\n?', '', resolved) | ||
| resolved = re.sub(r'=======\n?', '', resolved) | ||
| resolved = re.sub(r'>>>>>>> .*?\n?', '', resolved) | ||
|
|
||
| return resolved | ||
|
|
||
| def main(): | ||
| nga4_dir = Path('/home/thaianh/Study/GreenWave/src/backend/app/sumo_rl/sumo_files/Nga4ThuDuc') | ||
|
|
||
| print("🔍 Resolving Git merge conflicts in Nga4ThuDuc files...\n") | ||
|
|
||
| fixed_count = 0 | ||
|
|
||
| # Process all XML and sumocfg files | ||
| for pattern in ['*.xml', '*.sumocfg']: | ||
| for filepath in nga4_dir.glob(pattern): | ||
| with open(filepath, 'r', encoding='utf-8') as f: | ||
| content = f.read() | ||
|
|
||
| # Check if file has conflicts | ||
| if '<<<<<<< HEAD' in content: | ||
| print(f"🔧 Resolving {filepath.name}...") | ||
|
|
||
| # Resolve conflicts | ||
| resolved_content = resolve_conflict(content) | ||
|
|
||
| # Write back | ||
| with open(filepath, 'w', encoding='utf-8') as f: | ||
| f.write(resolved_content) | ||
|
|
||
| fixed_count += 1 | ||
| print(f" ✅ Fixed") | ||
| else: | ||
| print(f" ✓ {filepath.name} - No conflicts") | ||
|
|
||
| print(f"\n✅ Resolved conflicts in {fixed_count} files") | ||
|
|
||
| if __name__ == '__main__': | ||
| main() |
There was a problem hiding this comment.
[nitpick] This utility script should be placed in the scripts/ directory instead of the repository root to maintain better organization and consistency with other utility scripts like check_sumo.py and connect_sumo.py.
| #!/usr/bin/env python3 | ||
| """Fix XML files to have XML declaration at the beginning""" | ||
|
|
||
| import os | ||
| import re | ||
| from pathlib import Path | ||
|
|
||
| def fix_xml_file(filepath): | ||
| """Fix a single XML file""" | ||
| with open(filepath, 'r', encoding='utf-8') as f: | ||
| content = f.read() | ||
|
|
||
| # Check if file starts with XML declaration | ||
| if content.strip().startswith('<?xml'): | ||
| print(f"✓ {filepath.name} - Already correct") | ||
| return False | ||
|
|
||
| # Find the XML declaration | ||
| xml_decl_pattern = r'<\?xml[^?]*\?>' | ||
| match = re.search(xml_decl_pattern, content) | ||
|
|
||
| if not match: | ||
| print(f"✗ {filepath.name} - No XML declaration found") | ||
| return False | ||
|
|
||
| # Extract XML declaration | ||
| xml_decl = match.group(0) | ||
|
|
||
| # Remove the XML declaration from its current position | ||
| content_without_decl = content.replace(xml_decl, '', 1) | ||
|
|
||
| # Remove leading whitespace/newlines but keep copyright comments | ||
| content_without_decl = content_without_decl.lstrip('\n') | ||
|
|
||
| # Put XML declaration at the beginning | ||
| new_content = xml_decl + '\n' + content_without_decl | ||
|
|
||
| # Write back | ||
| with open(filepath, 'w', encoding='utf-8') as f: | ||
| f.write(new_content) | ||
|
|
||
| print(f"✓ {filepath.name} - Fixed") | ||
| return True | ||
|
|
||
| def main(): | ||
| # Path to Nga4ThuDuc directory | ||
| nga4_dir = Path('/home/thaianh/Study/GreenWave/src/backend/app/sumo_rl/sumo_files/Nga4ThuDuc') | ||
|
|
||
| print("Fixing XML files in Nga4ThuDuc directory...\n") | ||
|
|
||
| fixed_count = 0 | ||
|
|
||
| # Fix all .xml and .sumocfg files | ||
| for pattern in ['*.xml', '*.sumocfg']: | ||
| for filepath in nga4_dir.glob(pattern): | ||
| if fix_xml_file(filepath): | ||
| fixed_count += 1 | ||
|
|
||
| print(f"\nTotal files fixed: {fixed_count}") | ||
|
|
||
| if __name__ == '__main__': | ||
| main() |
There was a problem hiding this comment.
[nitpick] This utility script should be placed in the scripts/ directory instead of the repository root to maintain better organization and consistency with other utility scripts like check_sumo.py and connect_sumo.py.
| def fix_xml_file(filepath): | ||
| """Fix a single XML file""" | ||
| with open(filepath, 'r', encoding='utf-8') as f: | ||
| content = f.read() | ||
|
|
||
| # Check if file starts with XML declaration | ||
| if content.strip().startswith('<?xml'): | ||
| print(f"✓ {filepath.name} - Already correct") | ||
| return False | ||
|
|
||
| # Find the XML declaration | ||
| xml_decl_pattern = r'<\?xml[^?]*\?>' | ||
| match = re.search(xml_decl_pattern, content) | ||
|
|
||
| if not match: | ||
| print(f"✗ {filepath.name} - No XML declaration found") | ||
| return False | ||
|
|
||
| # Extract XML declaration | ||
| xml_decl = match.group(0) | ||
|
|
||
| # Remove the XML declaration from its current position | ||
| content_without_decl = content.replace(xml_decl, '', 1) | ||
|
|
||
| # Remove leading whitespace/newlines but keep copyright comments | ||
| content_without_decl = content_without_decl.lstrip('\n') | ||
|
|
||
| # Put XML declaration at the beginning | ||
| new_content = xml_decl + '\n' + content_without_decl | ||
|
|
||
| # Write back | ||
| with open(filepath, 'w', encoding='utf-8') as f: | ||
| f.write(new_content) | ||
|
|
||
| print(f"✓ {filepath.name} - Fixed") | ||
| return True |
There was a problem hiding this comment.
The function lacks proper error handling. If the file cannot be read, written, or if regex operations fail, the script will crash without helpful error messages. Consider adding try-except blocks and logging errors:
def fix_xml_file(filepath):
"""Fix a single XML file"""
try:
with open(filepath, 'r', encoding='utf-8') as f:
content = f.read()
except Exception as e:
print(f"✗ {filepath.name} - Error reading file: {e}")
return False
# ... rest of the logic ...
try:
with open(filepath, 'w', encoding='utf-8') as f:
f.write(new_content)
except Exception as e:
print(f"✗ {filepath.name} - Error writing file: {e}")
return False| #!/usr/bin/env python3 | ||
| """Fix XML files to have XML declaration at the beginning""" | ||
|
|
||
| import os |
There was a problem hiding this comment.
Import of 'os' is not used.
| import os |
| Keeps the HEAD version (current branch) and removes conflict markers | ||
| """ | ||
|
|
||
| import os |
There was a problem hiding this comment.
Import of 'os' is not used.
| import os |
Proposed changes
Fix errors from xml files cause by license
Types of changes
🐛 Bug fix (non-breaking change which fixes an issue)
✨ Feature (non-breaking change which adds functionality)
💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
📝 Docs (documentation only changes)
♻️ Refactor (code improvement without changing functionality)
Checklist
Docker build successful (src/backend)
All tests passing (pytest)
Code follows project standards (Black, Ruff, MyPy)
Service boundaries maintained
.env.example updated (if environment variables changed)
No debug print statements left