Update uart protocol to conform to avlos

[yconst]

Update uart protocol to conform to avlos

[preloop]

Preloop has reviewed this PR.

See the pinned summary comment below for the full review details and issue tracker.

[preloop]

🔍 Preloop Code Review

Last Updated: 2026-01-31 14:07:38Z
Reviewing Commit: 4b61888
Review Status: ❌ Changes Requested

---

📝 Summary

This PR migrates the UART protocol to a binary Avlos-style frame and reuses the generated endpoint dispatch. The overall direction looks good, but there are a couple of frame-validation issues that will break CRC checks and allow invalid payload lengths through.

✅ What Looks Good

• Clear transition to binary framing with CRC and endpoint dispatch reuse.
• UART low-level RX path is simplified and avoids ASCII protocol mixing.

---

⚠️ Issues Found

🟠 High Priority

• [Quality] CRC calculation excludes the last payload byte (CRC length uses len instead of len + 1) - firmware/src/uart/uart_interface.c:101
• [Quality] Payload length is not validated against the 8-byte buffer; oversized payloads pass uninitialized data to endpoints - firmware/src/uart/uart_interface.c:115

🟡 Medium Priority

• [Testing] No UART/Avlos frame parsing tests added for the new binary protocol (CRC, length bounds, read/write paths).

🟢 Low Priority

• [Quality] Response length is not bounded against UART_BYTE_LIMIT, risking TX buffer overflow if a callback returns an unexpected length - firmware/src/uart/uart_interface.c:58

---

✅ Resolved Issues

None yet.

---

📄 Documentation Impact

• README/docs: UART protocol description and framing details should be updated to match the new Avlos binary format.

---

Progress: 0 of 4 issues addressed

This summary updates automatically on each review. Inline comments provide detailed feedback on specific lines.

[preloop]

Preloop has reviewed this PR.

See the pinned summary comment below for the full review details and issue tracker.

[preloop]

🔍 Preloop Code Review

Last Updated: 2026-01-31 14:08:41 UTC
Reviewing Commit: 4b61888
Review Status: ❌ Changes Requested

---

📝 Summary

This PR refactors the UART path to a binary Avlos-style framing with CRC and ISR-driven TX/RX, which is a solid direction. However, CRC verification and payload-length validation issues will currently reject valid frames or accept malformed payloads, and the protocol change lacks test and documentation updates.

✅ What Looks Good

• Cleaned up UART initialization to the single in-use peripheral and IO mapping.
• CRC calculation via the hardware peripheral is efficient and consistent with embedded constraints.
• ISR TX/RX flow is simpler and easier to reason about than the prior ASCII parsing.

---

⚠️ Issues Found

🔴 Critical Issues

[Must be fixed before merge]

• None.

🟠 High Priority

[Should be addressed]

• [Quality] CRC verification uses the wrong length (drops valid frames) - firmware/src/uart/uart_interface.c:101

🟡 Medium Priority

[Suggestions for improvement]

• [Quality] Payload length is not validated before calling endpoints; uninitialized data can be used for writes - firmware/src/uart/uart_interface.c:117
• [Quality] Length byte only checked against buffer size, not protocol max/min - firmware/src/uart/uart_lowlevel.c:92
• [Testing] No UART protocol tests added for CRC, malformed frames, or read/write paths
• [Documentation] UART protocol change not reflected in user-facing docs

🟢 Low Priority

[Minor improvements, optional]

• None.

---

✅ Resolved Issues

None yet.

---

📄 Documentation Impact

• docs/interfaces/uart.rst: Update framing description to match binary Avlos + CRC format
• docs/protocol/overview.rst (or equivalent UART section): Note CRC requirement and payload limits

---

Progress: 0 of 5 issues addressed

This summary updates automatically on each review. Inline comments provide detailed feedback on specific lines.

[yconst]

Approved suggestions

[preloop]

Preloop has reviewed this PR.

See the pinned summary comment below for the full review details and issue tracker.

[preloop]

🔍 Preloop Code Review

Last Updated: 2026-01-31 14:31:32Z
Reviewing Commit: b8b4b65
Review Status: ❌ Changes Requested

---

📝 Summary

This PR migrates UART to the Avlos-style binary framing and reuses the Avlos endpoint dispatch. CRC length is corrected, but a duplicate buffer block currently breaks compilation and there are still guard/test/doc gaps to address.

✅ What Looks Good

• Clear binary framing with CRC and endpoint dispatch reuse.
• ISR-driven RX/TX path is simplified compared to the old ASCII protocol.

---

⚠️ Issues Found

🟠 High Priority

• [Quality] Duplicate buffer/data_len declarations in UART_process_message cause a compile error and bypass the validated buffer - firmware/src/uart/uart_interface.c:119

🟡 Medium Priority

• [Testing] No UART/Avlos frame parsing tests added (CRC, length bounds, read/write paths).
• [Documentation] UART protocol docs still describe the dot protocol and need the new binary Avlos framing.

🟢 Low Priority

• [Quality] Response length is not bounded against UART_BYTE_LIMIT, risking TX buffer overflow if a callback returns an unexpected length - firmware/src/uart/uart_interface.c:58

---

✅ Resolved Issues

• CRC verification length was off by one (len vs len + 1). - Fixed in b8b4b65
• Payload length validation was missing before endpoint dispatch. - Fixed in b8b4b65
• RX length byte needed protocol-bound validation at dispatch time. - Fixed in b8b4b65

---

📄 Documentation Impact

• docs/interfaces/interfaces.rst: Update UART protocol description to the binary Avlos framing (STX/LEN/CRC) and payload limits.

---

Progress: 3 of 7 issues addressed

This summary updates automatically on each review. Inline comments provide detailed feedback on specific lines.

[preloop]

🔍 Preloop Code Review

Last Updated: 2026-01-31 14:36:27Z
Reviewing Commit: b8b4b65
Review Status: 💬 Changes Suggested

---

📝 Summary

This PR completes the shift to an Avlos-style binary UART frame and reuses the generated endpoint dispatch. The core parsing/CRC fixes look good, but test coverage and documentation updates are still missing, and there’s still a small TX-length guard to add.

✅ What Looks Good

• Clean binary framing with CRC and Avlos endpoint reuse.
• RX/TX ISR flow is simpler and more deterministic than the old ASCII parsing.

---

⚠️ Issues Found

🟡 Medium Priority

[Suggestions for improvement]

• [Testing] No UART/Avlos frame parsing tests added (CRC, length bounds, read/write paths)
• [Documentation] UART protocol docs still describe the dot protocol and need the binary Avlos framing - docs/interfaces/interfaces.rst:27

🟢 Low Priority

[Minor improvements, optional]

• [Quality] Response length is not bounded against UART_BYTE_LIMIT, risking TX buffer overflow if a callback returns an unexpected length - firmware/src/uart/uart_interface.c:58

---

✅ Resolved Issues

• CRC verification length was off by one (len vs len + 1). - Fixed in b8b4b65
• Payload length validation was missing before endpoint dispatch. - Fixed in b8b4b65
• RX length byte needed protocol-bound validation at dispatch time. - Fixed in b8b4b65
• Duplicate buffer/data_len declarations caused a compile error. - Fixed in b8b4b65

---

📄 Documentation Impact

• docs/interfaces/interfaces.rst: Replace dot-protocol description with the Avlos binary framing (STX/LEN/CRC) and payload limits.

---

Progress: 4 of 7 issues addressed

This summary updates automatically on each review. Inline comments provide detailed feedback on specific lines.

[preloop]

Preloop has reviewed this PR.

See the pinned summary comment below for the full review details and issue tracker.

[yconst]

Resolved comments

Fixed

[yconst]

Following extensive testing with client side Studio this presented several bugs that are hard to diagnose. Closing for now.