Use correct data from DS for ping and lost packets status

[ThadHouse]

Summary by CodeRabbit

• Improvements

  • Improved UDP handling with a larger read buffer, read-count validation, and clearer logging.
  • Enhanced parsing of driver station packets to track missed-packet counts and robot trip timing.
  • Match-start now reliably resets missed-packet tracking.

• Behavior Change

  • Certain TCP status-style packets are now emitted as log events instead of being decoded into detailed status.

• Tests

  • Removed test that validated status-packet decoding (coverage reduced).

[coderabbitai]

Warning

Rate limit exceeded

@ThadHouse has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 24 minutes and 40 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Removed DS status decoding and missedPacketOffset; expanded UDP read/validation and tag-encoded parsing (tag 1 length 6) to update MissedPacketCount and DsRobotTripTimeMs; TCP packet-type-22 handling now emits a DS log event; signalMatchStart resets MissedPacketCount.

Changes

Cohort / File(s)	Summary
Driver Station Connection field/driver_station_connection.go	Removed missedPacketOffset field and decodeStatusPacket(); increased UDP read buffer and added length/count validation; parse DS tag-encoded UDP data (tag 1 length 6) to set MissedPacketCount and DsRobotTripTimeMs; TCP packet-type-22 now logs a DS event rather than decoding status; signalMatchStart resets MissedPacketCount.
Tests field/driver_station_connection_test.go	Removed TestDecodeStatusPacket and its assertions; import block reordered; status-packet decoding coverage removed.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant UDP as UDP Reader
participant F as Field / DriverStationConnection
participant TCP as TCP Handler
participant Log as Logger
UDP->>F: read packet (buf 1500) with count
alt packet contains tag 1 (len 6)
F->>F: parse MissedPacketCount, DsRobotTripTimeMs
else insufficient/short packet
F->>Log: log insufficient packet
end
TCP->>F: receive packet type 22
F->>Log: emit DS log event

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 I nibbled bytes in morning light,
UDP tags twitched, packets took flight,
Offsets gone, missed counts reset,
Logs now hum where status sat,
A hop, a whisker, code delight! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: refactoring DS (Driver Station) UDP packet handling to extract ping and lost packet data from tag-encoded fields instead of a status packet, improving data correctness.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

field/driver_station_connection.go (1)

104-108: Discarded UDP read error can cause a hot spin loop.

If listener.Read returns a persistent error (e.g., socket closed), the error is silently ignored and count will be 0, causing the count < 8 branch to log continuously in a tight loop. Consider checking the error and breaking or sleeping on non-transient errors.

Suggested handling

-		count, _ := listener.Read(data[:])
-		if count < 8 {
-			log.Printf("Received packet with insufficient length: %d", count)
+		count, err := listener.Read(data[:])
+		if err != nil {
+			log.Printf("Error reading from driver station UDP socket: %v", err)
+			continue
+		}
+		if count < 8 {
+			log.Printf("Received packet with insufficient length: %d", count)

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

field/driver_station_connection_test.go (1)

202-211: ⚠️ Potential issue | 🔴 Critical

Test is broken — assertions rely on removed TCP status-decoding path (CI failure).

The production code no longer decodes MissedPacketCount / DsRobotTripTimeMs from TCP packet type 22; these are now sourced from UDP tag-encoded data. The assertions on lines 209-210 will always see 0, which matches the pipeline failure:

expected: 103, actual: 0; expected: 14, actual: 0

Either remove these two assertions (and the stale comment on line 202) or replace them with a UDP-based test that sends a packet containing a tag-1 payload to the driverStationUdpReceivePort listener.

Minimal fix — remove stale assertions

-			// Check that an unknown packet type gets ignored and a status packet gets decoded.
+			// Check that an unknown packet type gets ignored.
 			dataSend = [5]byte{0, 3, 37, 0, 0}
 			tcpConn.Write(dataSend[:])
 			time.Sleep(time.Millisecond * 10)
-			dataSend2 := [38]byte{0, 36, 22, 28, 103, 19, 192, 0, 246}
-			tcpConn.Write(dataSend2[:])
-			time.Sleep(time.Millisecond * 10)
-			assert.Equal(t, 103, dsConn.MissedPacketCount)
-			assert.Equal(t, 14, dsConn.DsRobotTripTimeMs)

Ideally, add a dedicated test that exercises the new UDP tag-parsing path (send a crafted UDP packet with tag 1, length 6, and verify MissedPacketCount and DsRobotTripTimeMs are updated).

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@field/driver_station_connection.go`:
- Around line 104-108: The loop currently ignores the error returned by
listener.Read (call at listener.Read(data[:])) and only checks count, which can
cause an infinite spin/log spam if the socket is closed; update the read
handling in the same function so you capture the returned err alongside count,
check if err != nil, treat temporary/net.Error with Temporary() as a retry (log
and continue), and for permanent errors (including EOF or non-temporary errors)
log the error and break/return (or call log.Fatalf if appropriate) instead of
continuing; keep the existing insufficient-length check for count < 8 after
handling err.