Skip to content

Conversation

@PedroDeSanti
Copy link
Member

@PedroDeSanti PedroDeSanti commented Apr 28, 2025

Przinha que implementa um servico de comunicacao bluetooth

  • Basicamente é bem parecido com o storage, eu fiz uma interface ISerialVariable que extende o ISerializable colocando coisas inportantes como nome do tipo da variavel, nome da variavel e id
  • Tem duas classes que implementam essa interface, o CustomSerialVariable e o PrimitiveSerialVariable
  • Tem um SerialVariablePool que vai adicionar referencias as varaivei em um mapa
  • Um logger pra mandar mg de string
  • Um CommunicationService que recebe um logger e um SerialVariablePool e automaticamente fica mandando ou recebendo essas variaveis (pelo bluetooth no nosso caso

A forma como o CommunicationService é basicamente assim:

  • O cliente (webapp) vai se conectar no micras e mandar um request de mapa de variaveis, pra saber quais sao todas as variaveis que serão sincronizadas entre os dois, e as respectivas informacoes delas (id, nome, tipo da variavel)
  • A partir dai o micras e o cliente apensa se comunicarão com uma msg que contem um id e um payload
  • O cliente vai mandar para o micras as variaveis write_only e o micras vai desserializar o conteudo delas encaminhando para a variavel com id certo
  • O micras vai ficar mandando todas as variáveis read_only pro cliente

GabrielCosme and others added 30 commits September 30, 2024 05:52
Co-authored-by: Eduardo Santos Barreto <[email protected]>
Base automatically changed from feature/new-architecture to main April 30, 2025 15:26
@PedroDeSanti PedroDeSanti marked this pull request as ready for review April 30, 2025 15:28
@PedroDeSanti PedroDeSanti requested a review from Copilot April 30, 2025 15:36
Copy link

Copilot AI left a 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() {

Comment on lines +34 to +36
if (Packet::is_valid(packet_data)) { // check is redundant
this->incoming_packets.emplace(packet_data);
}
Copy link

Copilot AI Apr 30, 2025

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.

Suggested change
if (Packet::is_valid(packet_data)) { // check is redundant
this->incoming_packets.emplace(packet_data);
}
this->incoming_packets.emplace(packet_data);

Copilot uses AI. Check for mistakes.
Comment on lines +57 to +59
}

queue.pop_front();
Copy link

Copilot AI Apr 30, 2025

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.

Suggested change
}
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();
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

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.
*/
Copy link

Copilot AI Apr 30, 2025

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.

Suggested change
*/
*/
// 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.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

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

@PedroDeSanti PedroDeSanti force-pushed the feature/new-bluetooth branch from aec5462 to 44e9748 Compare May 2, 2025 23:57
@GabrielCosme GabrielCosme changed the base branch from main to feature/lint-target May 4, 2025 18:46
@GabrielCosme GabrielCosme reopened this May 5, 2025
@PedroDeSanti PedroDeSanti marked this pull request as draft May 5, 2025 00:01
Base automatically changed from feature/lint-target to main May 8, 2025 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants