Skip to content
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

Proc-macros: useless conversion Clippy lint #248

Open
ivmarkov opened this issue Jan 17, 2025 · 6 comments
Open

Proc-macros: useless conversion Clippy lint #248

ivmarkov opened this issue Jan 17, 2025 · 6 comments
Assignees

Comments

@ivmarkov
Copy link

Consider:

/// Matter service
#[gatt_service(uuid = MATTER_BLE_SERVICE_UUID16)]
struct MatterService {
    #[characteristic(uuid = Uuid::Uuid128(C1_CHARACTERISTIC_UUID.to_be_bytes()), write)]
    c1: External,
    #[characteristic(uuid = Uuid::Uuid128(C2_CHARACTERISTIC_UUID.to_be_bytes()), write, indicate)]
    c2: External,
}

... where, inside rs-matter, I have:

pub const MATTER_BLE_SERVICE_UUID16: u16 = 0xFFF6;
pub const C1_CHARACTERISTIC_UUID: u128 = 0x18EE2EF5263D4559959F4F9C429F9D11;
pub const C2_CHARACTERISTIC_UUID: u128 = 0x18EE2EF5263D4559959F4F9C429F9D12;

With MATTER_BLE_SERVICE_UUID16 everything is great, because trouble's Uuid has impl From<u16> for Uuid.
With c1 and c2 life is not so great, because Uuid has neither impl From<[u8; 16]> for Uuid, nor impl From<u128> for Uuid.

Therefore, I have to use the explicit Uuid::Uuid128(C1_CHARACTERISTIC_UUID.to_be_bytes()).
However, since the proc-macro generated code does internally do .into() on my Uuid, Clippy then kicks-in with:

warning: useless conversion to the same type: `trouble_host::prelude::Uuid`
  --> src/ble.rs:45:29
   |
45 |     #[characteristic(uuid = Uuid::Uuid128(C1_CHARACTERISTIC_UUID.to_be_bytes()), write)]
   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `Uuid::Uuid128(C1_CHARACTERISTIC_UUID.to_be_bytes())`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
   = note: `#[warn(clippy::useless_conversion)]` on by default

I think we either need extra From<???> for Uuid impls, or this (and other) Clippy lints disabled in the proc-macros, or ideally both.

@lure23
Copy link
Contributor

lure23 commented Jan 17, 2025

When making custom profiles, I think 128-bit UUID:s is the only way to go. Thus, really would like to have them supported and tested the same as 16-bit.

@jamessizeland
Copy link
Collaborator

Yep, makes sense, I can add that in too. So far I haven't because we support string literals for 128bit UUIDs and that's been enough for me so far.

But adding From is possible too, or we can add more types that impl Into<trouble_host::Uuid>

@ivmarkov
Copy link
Author

I think adding From<[u8; 16]> and From<u128> would do for now.

@ivmarkov
Copy link
Author

ivmarkov commented Jan 17, 2025

Perhaps also From<[u8; 2]> for symmetry.

@lure23
Copy link
Contributor

lure23 commented Jan 22, 2025

One notion regarding this: source

pub enum Uuid {
    Uuid16(u16),
    Uuid128([u8; 16]),
}

Is there a reason the source uses [u8; 16] and not u128 straight up? Since the latter is supported in Rust, wouldn't it be more aligned with the use of u16?

@jamessizeland
Copy link
Collaborator

I'm going to take a look at this now.

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

No branches or pull requests

3 participants