Skip to content

Conversation

caveman99
Copy link
Member

crafting a data structure for inter device link

@caveman99 caveman99 requested a review from Copilot July 2, 2025 21:52
Copy link
Contributor

@Copilot 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 refactors inter-device messaging by removing the old MessageType/SensorData structure and introducing new I²C command/response messages alongside a simple beep command.

  • Removed legacy MessageType enum and SensorData message
  • Added I2CCommand and I2CResponse messages
  • Introduced beep field in InterdeviceMessage and configured its size in options

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
meshtastic/interdevice.proto Removed MessageType/SensorData, added I2CCommand, I2CResponse, and beep field
meshtastic/interdevice.options Set beep field size to 16-bit
Comments suppressed due to low confidence (6)

meshtastic/interdevice.proto:11

  • Since the old MessageType enum was removed, reserve its previous tag numbers (e.g., 160–181) at the top of the .proto to prevent future reuse and maintain compatibility.
message I2CCommand {

meshtastic/interdevice.proto:11

  • Add a comment above I2CCommand and inline comments for its fields (op, addr, data, ack) to clarify their roles and units.
message I2CCommand {

meshtastic/interdevice.proto:12

  • Introduce a default UNSPECIFIED = 0 enum value so that the zero value doesn’t unintentionally map to START when unset.
  enum Operation {

meshtastic/interdevice.proto:21

  • [nitpick] The field name ack is ambiguous; consider renaming it to ack_required or expect_ack to clarify its meaning.
  bool ack = 4;

meshtastic/interdevice.proto:41

  • [nitpick] Rename beep to something more descriptive like beep_duration_ms or add a comment explaining its unit and purpose.
    uint32 beep = 4;

meshtastic/interdevice.proto:11

  • Add unit tests for I2CCommand and I2CResponse encoding/decoding to validate interoperability and catch future regressions.
message I2CCommand {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant