-
Notifications
You must be signed in to change notification settings - Fork 0
✨ Adiciona serviço de bluetooth #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Eduardo Santos Barreto <[email protected]>
…rasFirmware into feature/new-bluetooth
… Serializable concepts
…_ptr and adjust test_comm_service initialization
…ture/new-bluetooth
There was a problem hiding this 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 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); | ||
| } |
Copilot
AI
Apr 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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(); |
Copilot
AI
Apr 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discordo meu mano copiloto
| * @brief Get the variable's size. | ||
| * | ||
| * @return uint16_t size of the variable. | ||
| */ |
Copilot
AI
Apr 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <[email protected]>
Przinha que implementa um servico de comunicacao bluetooth
A forma como o CommunicationService é basicamente assim: