Conversation
Co-authored-by: Eduardo Santos Barreto <eduardopontobarreto@gmail.com>
…rasFirmware into feature/new-bluetooth
… Serializable concepts
…_ptr and adjust test_comm_service initialization
…ture/new-bluetooth
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new Bluetooth communication service along with associated interfaces and helper classes to serialize and manage variables, following a pattern similar to the existing storage service. Key changes include:
- Creation of a new UART DMA handler and logger to support Bluetooth functionality.
- Implementation of a CommunicationService handling incoming/outgoing packets, variable serialization, and log processing.
- Updates to configuration files and proxy interfaces to integrate the new Bluetooth service.
Reviewed Changes
Copilot reviewed 24 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| micras_hal/include/micras/hal/uart_dma.hpp | New UART DMA class for Bluetooth communication support. |
| micras_core/include/micras/core/utils.hpp | Addition of a compile-time type name utility. |
| micras_comm/src/packet.cpp | Packet serialization/deserialization improvements. |
| micras_comm/src/communication_service.cpp | New CommunicationService handling packet flow and logging. |
| micras_comm/include/micras/comm/vars/*.hpp | New variable serialization interfaces and their implementations. |
| include/micras/micras.hpp | Updated proxy list with the new Bluetooth interface. |
| config/target.hpp | Bluetooth config definitions added. |
| .github/workflows/*.yaml | Workflow updates; minor duplicate attribute declarations. |
Files not reviewed (2)
- CMakeLists.txt: Language not supported
- micras_comm/CMakeLists.txt: Language not supported
Comments suppressed due to low confidence (1)
micras_comm/src/communication_service.cpp:66
- The function name 'process_incomming_packets' contains a typo; consider renaming it to 'process_incoming_packets' for clarity.
void CommunicationService::process_incomming_packets() {
| if (Packet::is_valid(packet_data)) { // check is redundant | ||
| this->incoming_packets.emplace(packet_data); | ||
| } |
There was a problem hiding this comment.
The comment notes that the validity check is redundant; consider removing this extra check if Packet::is_valid is already ensured upstream to simplify the code.
| if (Packet::is_valid(packet_data)) { // check is redundant | |
| this->incoming_packets.emplace(packet_data); | |
| } | |
| this->incoming_packets.emplace(packet_data); |
| } | ||
|
|
||
| queue.pop_front(); |
There was a problem hiding this comment.
Review the conditional logic in extract_valid_packet: popping an extra element when encountering an escape byte might inadvertently skip a valid header; consider revising this logic to ensure correct packet extraction.
| } | |
| queue.pop_front(); | |
| if (!queue.empty() && queue.front() == Packet::header_byte) { | |
| break; // Stop if the next byte is a valid header | |
| } | |
| } | |
| if (!queue.empty()) { | |
| queue.pop_front(); | |
| } |
There was a problem hiding this comment.
discordo meu mano copiloto
| * @brief Get the variable's size. | ||
| * | ||
| * @return uint16_t size of the variable. | ||
| */ |
There was a problem hiding this comment.
[nitpick] Returning 0 for get_size() may be intentional; adding a comment to clarify why the size is fixed to 0 for custom serial variables would help future maintainers.
| */ | |
| */ | |
| // The size is fixed to 0 because the actual size of the custom serializable variable | |
| // is not relevant in this context. This method is implemented to satisfy the interface | |
| // requirements and does not impact serialization or deserialization functionality. |
There was a problem hiding this comment.
se pa vou tirar esse metodo de get_size pq nao to usando
aec5462 to
44e9748
Compare
…ture/new-bluetooth
…rmware into feature/new-bluetooth
Co-authored-by: Pedro de Santi <PedroDeSanti@users.noreply.github.com>
Przinha que implementa um servico de comunicacao bluetooth
A forma como o CommunicationService é basicamente assim: