Skip to content

Conversation

@Cantonplas
Copy link
Contributor

Now all sockets and packet period time will be described through adj, allowing the possibility of autogenerating all the code

Copy link

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 pull request introduces a mechanism to autogenerate socket and packet transmission configuration through the ADJ (Adjacency) system. The changes add socket definitions and packet timing information to enable code generation for network communication.

Changes:

  • Added new sockets.json file defining network socket configurations (ServerSocket, DatagramSocket, and Socket types) for VCU board
  • Enhanced packet definitions with transmission period (period, period_type) and target socket (socket) fields
  • Updated board configuration to reference the new sockets definition file
  • Extended README documentation with socket schema, field descriptions, and usage examples

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 17 comments.

File Description
boards/VCU/sockets.json New file defining 6 network sockets (3 TCP, 3 UDP) for control station, PCU, and HVSCU communication
boards/VCU/packets.json Added period timing and socket routing information to 8 data packets
boards/VCU/VCU.json Integrated sockets configuration file reference into board configuration
README.md Added comprehensive documentation for sockets schema, updated packet schema with new fields, and added configuration rules

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


5. **ID Uniqueness**: Board IDs must be unique across all boards. Packet IDs should be unique within their type category.

6. **Socket Naming**: The sockets referenced inside the packet definitions must use exact names as defined in `socket.json`.
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

The reference should be "sockets.json" (plural) to match the actual filename, not "socket.json".

Suggested change
6. **Socket Naming**: The sockets referenced inside the packet definitions must use exact names as defined in `socket.json`.
6. **Socket Naming**: The sockets referenced inside the packet definitions must use exact names as defined in `sockets.json`.

Copilot uses AI. Check for mistakes.
README.md Outdated
"name": "Brake Telemetry",
"variables": ["brake_pressure", "brake_status", "brake_temperature"]
"variables": ["brake_pressure", "brake_status", "brake_temperature"],
"period_ms": 16.67,
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

The example uses "period_ms" as a field name, but the actual implementation uses two separate fields: "period_type" and "period". The example should be updated to match the actual schema being used throughout packets.json.

Suggested change
"period_ms": 16.67,
"period_type": "milliseconds",
"period": 16.67,

Copilot uses AI. Check for mistakes.
{
"type": "DatagramSocket",
"name": "hvscu_udp",
"remote_ip":"192.168.1.7",
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

Inconsistent spacing after colons in JSON. Line 46 has "remote_ip":"192.168.1.7" without a space after the colon, while other fields have spaces after colons. For consistency, consider adding a space after the colon.

Copilot uses AI. Check for mistakes.
Comment on lines +220 to +229
```json
{
"type": "string",
"name": "string",
"port": "number?",
"local_port":"number?",
"remote_ip": "string?",
"remote_port":"number?"

}
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

The schema shows a single socket object, but the actual sockets.json file contains an array of socket objects. The schema should be wrapped in array brackets to accurately represent the structure. Change the opening brace on line 221 to an opening bracket and update line 229 accordingly.

Copilot uses AI. Check for mistakes.
Comment on lines +49 to +50
"period_type": "us",
"period": 16.67,
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

The period is set to 16.67 with period_type "us" (microseconds). This seems inconsistent with the other packets that use "ms" (milliseconds) with the same period value of 16.67. A period of 16.67 microseconds would be extremely fast (approximately 60,000 Hz). This is likely intended to be "ms" for a ~60 Hz update rate, consistent with the other packets.

Copilot uses AI. Check for mistakes.
```

**Field Descriptions:**
- `type`: Socket type string(e.g., "ServerSocket","DatagramSocket","Socket")
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

Missing space after colon in the field description. Should be "string (e.g.," with a space after the colon instead of "string(e.g.,".

Suggested change
- `type`: Socket type string(e.g., "ServerSocket","DatagramSocket","Socket")
- `type`: Socket type string (e.g., "ServerSocket","DatagramSocket","Socket")

Copilot uses AI. Check for mistakes.
- `period_type`: Optional string specifying the type of measurement for period, either microseconds
or milliseconds
- `period`: Optional number specifying the transmission period in milliseconds or microseconds for periodic packets. Can be an integer or floating-point value
- `socket`: Optinal string that should match the name of the socket you want to transmit this packet trhough
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

The word "Optinal" should be spelled "Optional".

Suggested change
- `socket`: Optinal string that should match the name of the socket you want to transmit this packet trhough
- `socket`: Optional string that should match the name of the socket you want to transmit this packet trhough

Copilot uses AI. Check for mistakes.
{
"type": "DatagramSocket",
"name": "control_station_udp",
"remote_ip":"192.168.0.9",
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

Inconsistent spacing after colons in the example JSON. Line 252 has "remote_ip":"192.168.0.9" without a space after the colon, while other fields in the example have spaces. For consistency, add a space after the colon.

Suggested change
"remote_ip":"192.168.0.9",
"remote_ip": "192.168.0.9",

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +54





Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

Excessive blank lines at the end of the file. Lines 50-54 contain only whitespace and should be removed for cleaner formatting.

Suggested change

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +40


Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

Blank lines 39-40 appear to contain only whitespace. Consider removing these extra blank lines for cleaner formatting.

Copilot uses AI. Check for mistakes.
@JavierRibaldelRio
Copy link
Contributor

ADJ-Validator, now suports this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants