Merge pull request #15 from canardleteer/dev/canardleteer/code-cleanup

Code Hygiene Job + Misc clippy fixes.

.github/workflows/hygiene.yml

@@ -0,0 +1,44 @@
+on: [push, pull_request]
+
+name: "Code Hygiene Suite"
+
+env:
+    CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
+
+jobs:
+  hygiene:
+    strategy:
+      matrix:
+        platform: [macos-latest, ubuntu-20.04, windows-latest]
+
+    runs-on: ${{ matrix.platform }}
+
+    steps:
+    - uses: actions/checkout@v4
+      with:
+        submodules: true
+
+    - name: Install dependencies (Ubuntu only)
+      if: matrix.platform == 'ubuntu-20.04'
+      run: |
+          sudo apt-get update
+          sudo apt-get install -y libdbus-1-dev pkg-config
+
+    - name: rust toolchain
+      uses: dtolnay/rust-toolchain@stable
+
+    - name: Initialize Rust Cache
+      uses: actions/cache@v4
+      with:
+        path: |
+          ~/target
+        key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
+
+    - name: cargo check
+      run: cargo check
+
+    - name: cargo fmt
+      run: cargo fmt --all -- --check
+
+    - name: cargo clippy
+      run: cargo clippy -- -D warnings

.github/workflows/testing.yml

@@ -11,11 +11,12 @@ jobs:
 
     runs-on: ${{ matrix.platform }}
     steps:
-      - uses: actions/checkout@v3
-      - run: git submodule update --init
+      - uses: actions/checkout@v4
+        with:
+          submodules: true
 
       - name: Initialize Rust Cache
-        uses: actions/cache@v2
+        uses: actions/cache@v4
         with:
           path: |
             ~/target

build.rs

@@ -55,6 +55,7 @@ fn main() -> std::io::Result<()> {
     #[cfg(feature = "serde")]
     {
         config.type_attribute(".", "#[serde(rename_all = \"camelCase\")]");
+        config.type_attribute(".", "#[allow(clippy::doc_lazy_continuation)]");
     }
 
     config.compile_protos(&protos, &[protobufs_dir]).unwrap();

src/connections/mod.rs

@@ -18,7 +18,7 @@ pub mod wrappers;
 /// * `Local` - A packet that should be handled by the connected node.
 /// * `Broadcast` - A packet that should be broadcast to all nodes in the mesh.
 /// * `Node(u32)` - A packet that should be sent to a specific node in the mesh,
-/// specified by the passed `u32` id.
+///     specified by the passed `u32` id.
 ///
 /// # Default
 ///

src/connections/stream_api.rs

@@ -112,11 +112,11 @@ impl<State> ConnectedStreamApi<State> {
     /// * `destination` - A `PacketDestination` enum that specifies the destination of the packet.
     /// * `channel` - A `u32` that specifies the message channel to send the packet on, in the range [0..7).
     /// * `want_ack` - A `bool` that specifies whether or not the radio should wait for acknowledgement
-    /// from other nodes on the mesh.
+    ///     from other nodes on the mesh.
     /// * `want_response` - A `bool` that specifies whether or not the radio should wait for a response
-    /// from other nodes on the mesh.
+    ///     from other nodes on the mesh.
     /// * `echo_response` - A `bool` that specifies whether or not the radio should echo the packet back
-    /// to the client.
+    ///     to the client.
     /// * `reply_id` - An optional `u32` that specifies the ID of the packet to reply to.
     /// * `emoji` - An optional `u32` that specifies the unicode emoji data to send with the packet.
     ///
@@ -179,6 +179,10 @@ impl<State> ConnectedStreamApi<State> {
             PacketDestination::Node(id) => id,
         };
 
+        // NOTE(canardleteer): We don't warn on deprecation here, because it
+        //                     remains valid for many active nodes, and
+        //                     remains a part of the generated interface.
+        #[allow(deprecated)]
         let mut mesh_packet = protobufs::MeshPacket {
             payload_variant: Some(protobufs::mesh_packet::PayloadVariant::Decoded(
                 protobufs::Data {
@@ -197,7 +201,7 @@ impl<State> ConnectedStreamApi<State> {
             hop_limit: 0, // * not transmitted
             priority: 0,  // * not transmitted
             rx_rssi: 0,   // * not transmitted
-            delayed: 0,   // * not transmitted
+            delayed: 0,   // * not transmitted [deprecated since protobufs v2.2.19]
             hop_start: 0, // * set on device
             via_mqtt: false,
             from: own_node_id.id(),
@@ -495,7 +499,7 @@ impl ConnectedStreamApi<state::Connected> {
     /// # Arguments
     ///
     /// * `config_id` - A randomly generated configuration ID that will be used
-    /// to check that the configuration process has completed.
+    ///     to check that the configuration process has completed.
     ///
     /// # Returns
     ///
@@ -617,11 +621,11 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `text` - A `String` containing the text to send.
     /// * `destination` - A `PacketDestination` enum that specifies the destination of the packet.
     /// * `want_ack` - A `bool` that specifies whether or not the radio should wait for acknowledgement
-    /// from other nodes on the mesh.
+    ///     from other nodes on the mesh.
     /// * `channel` - A `u32` that specifies the message channel to send the packet on [0..7).
     ///
     /// # Returns
@@ -689,11 +693,11 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `waypoint` - An instance of the `Waypoint` struct to send.
     /// * `destination` - A `PacketDestination` enum that specifies the destination of the packet.
     /// * `want_ack` - A `bool` that specifies whether or not the radio should wait for acknowledgement
-    /// from other nodes on the mesh.
+    ///     from other nodes on the mesh.
     /// * `channel` - A `u32` that specifies the message channel to send the packet on [0..7).
     ///
     /// # Returns
@@ -770,11 +774,11 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `position` - An instance of the `Position` struct to send.
     /// * `destination` - A `PacketDestination` enum that specifies the destination of the packet.
     /// * `want_ack` - A `bool` that specifies whether or not the radio should wait for acknowledgement
-    /// from other nodes on the mesh.
+    ///     from other nodes on the mesh.
     /// * `channel` - A `u32` that specifies the message channel to send the packet on [0..7).
     ///
     /// # Returns
@@ -847,7 +851,7 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `config` - An instance of the `Config` struct to update the radio with.
     ///
     /// # Returns
@@ -921,7 +925,7 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `module_config` - An instance of the `ModuleConfig` struct to update the radio with.
     ///
     /// # Returns
@@ -995,7 +999,7 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `channel_config` - An instance of the `Channel` struct to update the radio with.
     ///
     /// # Returns
@@ -1066,7 +1070,7 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `user` - An instance of the `User` struct to update the radio user with.
     ///
     /// # Returns
@@ -1267,7 +1271,7 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `local_config` - An instance of the `LocalConfig` struct to update the radio with.
     ///
     /// # Returns
@@ -1382,7 +1386,7 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `local_module_config` - An instance of the `LocalModuleConfig` struct to update the radio with.
     ///
     /// # Returns
@@ -1525,7 +1529,7 @@ impl ConnectedStreamApi<state::Configured> {
     /// # Arguments
     ///
     /// * `packet_router` - A generic packet router field that implements the `PacketRouter` trait.
-    /// This router is used in the event a packet needs to be echoed.
+    ///     This router is used in the event a packet needs to be echoed.
     /// * `channel_config` - A list of updates to make to radio channels.
     ///
     /// # Returns

src/connections/stream_buffer.rs

@@ -238,14 +238,14 @@ impl StreamBuffer {
     // We need to also validate that, if the 0x94 is found and not at the end of the
     // buffer, that the next byte is 0xc3
     // Note that the maximum packet size currently stands at 240 bytes, meaning an MSB is not needed
-    fn find_framing_index(buffer: &mut Vec<u8>) -> Result<Option<usize>, StreamBufferError> {
+    fn find_framing_index(buffer: &mut [u8]) -> Result<Option<usize>, StreamBufferError> {
         // Not possible to have a two-byte sequence in a buffer with less than two bytes
         // Vec::windows will also panic if the buffer is empty
         if buffer.len() < 2 {
             return Ok(None);
         }
 
-        let framing_index = buffer.windows(2).position(|b| b == &[0x94, 0xc3]);
+        let framing_index = buffer.windows(2).position(|b| b == [0x94, 0xc3]);
 
         Ok(framing_index)
     }
@@ -293,7 +293,7 @@ impl StreamBuffer {
         // Recall that packet size doesn't include the first four magic bytes
         let incoming_packet_data_size: usize = usize::from(u16::from_le_bytes([*lsb, *msb]));
 
-        return Ok(incoming_packet_data_size);
+        Ok(incoming_packet_data_size)
     }
 
     fn validate_packet_in_buffer(
@@ -314,7 +314,7 @@ impl StreamBuffer {
         // In the event that the last byte is 0x94, we need to account for the possibility of
         // the next byte being 0xc3, which would indicate that the packet is malformed.
         // We can only do this when the buffer has enough data to avoid a slice index panic.
-        if self.buffer.len() >= packet_data_end_index + 1 {
+        if self.buffer.len() > packet_data_end_index {
             packet_data_end_index += 1;
         }
 

src/lib.rs

@@ -48,12 +48,12 @@ pub mod errors {
 /// destinations for packets sent to the radio by the library:
 ///
 /// * `PacketDestination::Local` - This destination is used for packets that are intended to be processed locally
-/// by the radio and not to be forwarded to other nodes. An example of this would be local configuration packets.
+///     by the radio and not to be forwarded to other nodes. An example of this would be local configuration packets.
 /// * `PacketDestination::Broadcast` - This destination is used for packets that are intended to be broadcast to all
-/// nodes in the mesh. This is the default enum variant. Text messages are commonly broadcasted to the entire mesh.
+///     nodes in the mesh. This is the default enum variant. Text messages are commonly broadcasted to the entire mesh.
 /// * `PacketDestination::Node(u32)` - This destination is used for packets that are intended to be sent to a specific
-/// node in the mesh. The `u32` value is the node id of the node that the packet should be sent to. This is commonly
-/// used for direct text messages.
+///     node in the mesh. The `u32` value is the node id of the node that the packet should be sent to. This is commonly
+///     used for direct text messages.
 ///
 /// The `PacketRouter` trait defines the behavior of a struct that is able to route mesh packets. This trait is used
 /// to allow for the echoing of mesh packets within the `send_mesh_packet` method of the `ConnectedStreamApi` struct.

src/utils_internal.rs

@@ -1,6 +1,6 @@
-use crate::errors_internal::Error;
 #[cfg(feature = "bluetooth-le")]
 use crate::errors_internal::BleConnectionError;
+use crate::errors_internal::Error;
 #[cfg(feature = "bluetooth-le")]
 use btleplug::api::{Central, Manager as _, Peripheral as _, ScanFilter};
 #[cfg(feature = "bluetooth-le")]
@@ -79,12 +79,12 @@ pub fn available_serial_ports() -> Result<Vec<String>, tokio_serial::Error> {
 /// # Arguments
 ///
 /// * `port_name` - The system-specific name of the serial port to open. Unix ports
-/// will be of the form /dev/ttyUSBx, while Windows ports will be of the form COMx.
+///     will be of the form /dev/ttyUSBx, while Windows ports will be of the form COMx.
 /// * `baud_rate` - The baud rate of the serial port. Defaults to `115_200` if not passed.
 /// * `dtr` - Asserts the "Data Terminal Ready" signal for the serial port if `true`.
-/// Defaults to `true` if not passed.
+///     Defaults to `true` if not passed.
 /// * `rts` - Asserts the "Request To Send" signal for the serial port if `true`.
-/// Defaults to `false` if not passed.
+///     Defaults to `false` if not passed.
 ///
 /// # Returns
 ///
@@ -204,11 +204,12 @@ pub async fn build_tcp_stream(
     Ok(StreamHandle::from_stream(stream))
 }
 
-
 #[cfg(feature = "bluetooth-le")]
+#[allow(dead_code)]
 const MSH_SERVICE: Uuid = Uuid::from_u128(0x6ba1b218_15a8_461f_9fa8_5dcae273eafd);
 
 #[cfg(feature = "bluetooth-le")]
+#[allow(dead_code)]
 async fn scan_peripherals(adapter: &Adapter) -> Result<Vec<Peripheral>, btleplug::Error> {
     adapter
         .start_scan(ScanFilter {
@@ -221,6 +222,7 @@ async fn scan_peripherals(adapter: &Adapter) -> Result<Vec<Peripheral>, btleplug
 /// Finds a BLE radio matching a given name and running meshtastic.
 /// It searches for the 'MSH_SERVICE' running on the device.
 #[cfg(feature = "bluetooth-le")]
+#[allow(dead_code)]
 async fn find_ble_radio(name: String) -> Result<Peripheral, Error> {
     //TODO: support searching both by a name and by a MAC address
     let scan_error_fn = |e: btleplug::Error| Error::StreamBuildError {
@@ -231,7 +233,7 @@ async fn find_ble_radio(name: String) -> Result<Peripheral, Error> {
     let adapters = manager.adapters().await.map_err(scan_error_fn)?;
 
     for adapter in &adapters {
-        let peripherals = scan_peripherals(&adapter).await;
+        let peripherals = scan_peripherals(adapter).await;
         match peripherals {
             Err(e) => {
                 error!("Error while scanning for meshtastic peripherals: {e:?}");