Conversation
alx87grd
commented
Sep 23, 2026
alx87grd
commented
Collaborator
- small clean-up in teleop/controller python scripts
- New improved low-level communication from the Arduino to row
Give one signal catalog and minilink-style ctl(x, r, t, params) placeholders so students can paste K without changing the default open-loop zeros. Still needs a run on the real car. Co-authored-by: Cursor <cursoragent@cursor.com>
Mode 4 left a false speed spike on the next tick; the Hz comments did not match the loop tests. Co-authored-by: Cursor <cursoragent@cursor.com>
… storage. Topics without a header were stored in nanoseconds, mixed with header stamps in seconds. Co-authored-by: Cursor <cursoragent@cursor.com>
millis() on the Mega counts 1.024 ms ticks and skips a value every 42.7 ms, so an integer-ms dt_low makes the raw speed read 25 to 33 % low on some ticks and after each blocking publish. Swapping the 4 tagged lines on the car passes a float dt in ms measured with micros() to ctl(); the loop rate is unchanged. Comments only: the compiled behaviour is identical until the lines are swapped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…serial send. Test branch. The millis() time base gave a ~3.07 ms tick (not 2 ms) and speed dips of 25-33 %, and Serial.flush() plus a 64-byte TX buffer froze loop() ~14 ms per publish. Now: time base in us (2 ms tick, 25 ms publish, 1 s watchdog), float dt in ms to ctl(), data[8] still in ms for GRO830, no flush and a 256-byte TX buffer (platformio.ini). Debug: the PAUSE line publishes the sensorsCallback duration in data[7]; the PBSEND line switches to a pbSend that sends the same bytes without String/sprintf, for when that duration is too long. Host-tested only, no AVR build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Measured on a cycle-accurate ATmega2560 (simavr, 16 MHz) with realistic values: 9.7 ms for the String/sprintf pbSend and 1.8 ms for the fast one (0.4 ms encoding, 0.3 ms hex, 1.0 ms queuing the 161 chars), not the 0.2 ms first estimated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
validated 500 hz control loop and 50 hz com
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The teleop callback can access unvalidated buttons, and the optimized protobuf sender regresses multi-message frames.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (3)
What changed in this PR
Cleans up controller/teleoperation scripts and improves Arduino-to-ROS communication timing and throughput.
Changes:
- Updates joystick mappings, controller templates, and documentation.
- Adds faster protobuf serialization and non-blocking serial transmission.
- Improves timing units, odometry data, and rosbag timestamp handling.
| File | Description |
|---|---|
README.md |
Updates controller mode documentation. |
racecar_teleop/racecar_teleop/slash_teleop.py |
Cleans up teleoperation logic and remaps fixed-speed mode. |
racecar_autopilot/racecar_autopilot/wall_estimator.py |
Corrects a wall-estimation comment. |
racecar_autopilot/racecar_autopilot/slash_controller.py |
Restructures controller templates and parameters. |
racecar_autopilot/racecar_autopilot/rosbag2csv.py |
Adds storage auto-detection and consistent timestamps. |
racecar_arduino/Controller/src/PBUtils.cpp |
Adds optimized protobuf frame serialization. |
racecar_arduino/Controller/src/main.cpp |
Reworks control, communication, and watchdog timing. |
racecar_arduino/Controller/platformio.ini |
Enlarges the serial transmission buffer. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| void PBUtils::pbSend(int nbs, ...) | ||
| { | ||
| static const char hexDigits[] = "0123456789ABCDEF"; | ||
| char frame[2 * MAX_MSG_LEN + 16]; // "<" + id + "|" + 2 hex chars per byte + ";" + ">" (on the stack) |
Comment on lines
+41
to
+43
| self.get_logger().info( | ||
| "slash_teleop: Received topic doesn't have enough axes and/or buttons. If a Logitech gamepad is used, make sure also it is in X mode. Will not warn again." | ||
| ) |
Comment on lines
+113
to
+114
| 7|`LB` + `R3`| Empty Template | ||
| 8|`LB` + `Cross key Up/Down`| Empty Template |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.