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

Whats the recommended way to differentiate Characteristic handles? #82

Closed
Syphixs opened this issue Aug 21, 2024 · 12 comments · Fixed by #84
Closed

Whats the recommended way to differentiate Characteristic handles? #82

Syphixs opened this issue Aug 21, 2024 · 12 comments · Fixed by #84

Comments

@Syphixs
Copy link

Syphixs commented Aug 21, 2024

Lets assume I have two or more Characteristics and want do differentiate between them on an write event easily. Whats the recommended way of doing this as the 'handle' field of 'Characteristic' is private to outside the crate? Do i need to build an outside table to link the handle number back to my characteristics?

    let mut bat_level = [23; 1];
    let battery_handle = {
        let mut svc = table.add_service(Service::new(0x1800));
        let _ = svc.add_characteristic_ro(0x2a00, id);
        let _ = svc.add_characteristic_ro(0x2a01, &appearance[..]);
        svc.build();

        // Generic attribute service (mandatory)
        table.add_service(Service::new(0x1801));

        // Battery service
        let mut svc = table.add_service(Service::new(0x180f));

        svc.add_characteristic(
            0x2a19,
            &[CharacteristicProp::Read, CharacteristicProp::Notify],
            &mut bat_level,
        )
        .build()
    };

    // Add custom Data Service
    let mut data_buffer = [0u8; 40]; 
    let data_characteristic_handle = {
        let mut svc = table.add_service(Service::new(DATA_SERVICE_UUID));

        svc.add_characteristic(
            DATA_CHARACTERISTIC_UUID,
            &[
                CharacteristicProp::Read,
                CharacteristicProp::Write,
                CharacteristicProp::Notify,
            ],
            &mut data_buffer,
        )
        .build()
    };

    // Add custom Data Service2
    let mut data_buffer2 = [0u8; 40];
    let data_characteristic_handle2 = {
        let mut svc = table.add_service(Service::new(DATA_SERVICE_UUID));

        svc.add_characteristic(
            DATA_CHARACTERISTIC_UUID,
            &[
                CharacteristicProp::Read,
                CharacteristicProp::Write,
                CharacteristicProp::Notify,
            ],
            &mut data_buffer,
        )
        .build()
    };


    let server = ble.gatt_server(&table);
    let mut adv_data = [0; 31];
    // Advertisement packets have a limit of 31 bytes!
    unwrap!(AdStructure::encode_slice(
        &[
            AdStructure::Flags(LE_GENERAL_DISCOVERABLE | BR_EDR_NOT_SUPPORTED),
            AdStructure::ServiceUuids16(&[Uuid::Uuid16([0x0f, 0x18])]),
            AdStructure::ServiceUuids16(&[DATA_SERVICE_UUID]),
            AdStructure::CompleteLocalName(b"Trouble"),
        ],
        &mut adv_data[..],
    ));

    info!("Starting advertising and GATT service");
    let _ = join3(
        ble.run(),
        async {
            loop {
                match server.next().await {
                    Ok(GattEvent::Write {
                        handle,
                        connection: _,
                    }) => {
                        // not possible as .handle is private
                        // if handle == data_characteristic_handle.handle 
                        let _ = table.get(handle, |value| {
                            info!(
                                "Write event. Value written: {:?}  and handle {:?}",
                                value, handle
                            );
                            // This only provides the internal number of handle 
                            // eg. Write event. Value written: [170, ..]  and handle Characteristic { cccd_handle: None, handle: 50 }
                        });
                    }
                    Ok(GattEvent::Read {
                        handle,
                        connection: _,
                    }) => {
                        info!("Read event");
                    }
                    Err(e) => {
                        error!("Error processing GATT events: {:?}", e);
                    }
                }
            }
        },


@lulf
Copy link
Member

lulf commented Aug 21, 2024

Good question... I think one way could be to have the CharacteristiBuilder::build() return Characteristic rather than AttributeHandle, and if we add PartialEq on Characteristic it should be possible to check and match?

That way you can't construct handles that are not provided by trouble.

@Syphixs
Copy link
Author

Syphixs commented Aug 21, 2024

yeah that seems like a good idea. A match on handles would be great.

Regarding "That way you can't construct handles that are not provided by trouble." I am not sure if i understand correctly what you mean by that. Is there a usecase for handles which are not handled by trouble?

@lulf
Copy link
Member

lulf commented Aug 21, 2024

yeah that seems like a good idea. A match on handles would be great.

Regarding "That way you can't construct handles that are not provided by trouble." I am not sure if i understand correctly what you mean by that. Is there a usecase for handles which are not handled by trouble?

No I don't think there are any such use case.

I was just thinking out louad that if it exposed the u16 and you could compare that, you'd not be able to guarantee that you always compared with a valid handle, so a good thing.

@Syphixs
Copy link
Author

Syphixs commented Aug 22, 2024

Thank you very much for implementing a fix that fast! Sadly i still have problems as it seems the equal check does not work correctly. It always enters the first one no matter the characteristic I actually write to. But the handle id is indeed different so maybe comparing on this number would be an option? Although I am still a little be confused what that number actually represents and why it's incrementing that often.
eg.

...
    let mut bat_level = [23; 1];
    let battery_lvl_handle = {
        let mut svc = table.add_service(Service::new(0x1800));
        let _ = svc.add_characteristic_ro(0x2a00, id);
        let _ = svc.add_characteristic_ro(0x2a01, &appearance[..]);
        svc.build();

        // Generic attribute service (mandatory)
        table.add_service(Service::new(0x1801));

        // Battery service
        let mut svc = table.add_service(Service::new(0x180f));

        svc.add_characteristic(
            0x2a19,
            &[CharacteristicProp::Read, CharacteristicProp::Notify],
            &mut bat_level,
        )
        .build()
    };

    // Event Service for all events that happened on device
    let mut event_buffer = [0u8; 40]; // Adjust size as needed
    let events_handle = {
        let mut svc = table.add_service(Service::new(EVENT_SERVICE_UUID));

        svc.add_characteristic(
            EVENT_CHARACTERISTIC_UUID,
            &[
                CharacteristicProp::Read,
                CharacteristicProp::Write,
                CharacteristicProp::Notify,
            ],
            &mut event_buffer,
        )
        .build()
    };



    info!("Starting advertising and GATT service");
    let _ = join3(
        ble.run(),
        async {
            loop {
                match server.next().await {
                    Ok(GattEvent::Write {
                        handle,
                        connection: _,
                    }) => {
                        info!("HANDLE Info {:?}", handle);

                        if handle == battery_lvl_handle {
                        // always enters here although its not the correct handle
                            info!("is battery_lvl_handle");
                        } else if handle == events_handle {
                            info!("is events_handle");
                        }
                  
                    }
....

@Syphixs
Copy link
Author

Syphixs commented Aug 22, 2024

@lulf sadly cannot reopen issue myself :)

@lulf lulf reopened this Aug 22, 2024
@lulf
Copy link
Member

lulf commented Aug 22, 2024

@Syphixs I think this fixes the issue #88. It also adds a test case to check that handle in event matches what was created by table. The handles would not match earlier because the event handle didn't contain the cccd field.

It doesn't explain the fact that you do match on the battery_lvl_handle though, I found that strange.

@lulf
Copy link
Member

lulf commented Aug 22, 2024

Could you try again with latest main?

@Syphixs
Copy link
Author

Syphixs commented Aug 22, 2024

Yeah now it works great! Thank you! :)

maybe it had something to do with a strange warning I also got which was:

0.201141 INFO  [host] initialized
└─ trouble_host::host::{impl#2}::run_with_handler::{async_fn#0}::{async_block#0} @ /Users/u/.cargo/git/checkouts/trouble-148cf413cadab653/12645f7/host/src/fmt.rs:143
0.201293 TRACE [host] enabling advertising
└─ trouble_host::host::{impl#2}::advertise::{async_fn#0} @ /Users/u/.cargo/git/checkouts/trouble-148cf413cadab653/12645f7/host/src/fmt.rs:117
14.504638 TRACE [host] connection with handle ConnHandle(1) established to BdAddr([dc, 1d, b9, 7d, d9, 59])
└─ trouble_host::host::{impl#2}::handle_connection @ /Users/u/.cargo/git/checkouts/trouble-148cf413cadab653/12645f7/host/src/fmt.rs:117
14.504760 TRACE [link][poll_accept] connection handle ConnHandle(1) in role Peripheral accepted
└─ trouble_host::connection_manager::{impl#1}::poll_accept @ /Users/u/.cargo/git/checkouts/trouble-148cf413cadab653/12645f7/host/src/fmt.rs:117
15.007537 WARN  [host] unsupported l2cap channel id 58
└─ trouble_host::host::{impl#2}::handle_acl @ /Users/u/.cargo/git/checkouts/trouble-148cf413cadab653/12645f7/host/src/fmt.rs:156
15.007568 WARN  [host] encountered error processing ACL data for ConnHandle(1): NotSupported
└─ trouble_host::host::{impl#2}::run_with_handler::{async_fn#0}::{async_block#2} @ /Users/u/.cargo/git/checkouts/trouble-148cf413cadab653/12645f7/host/src/fmt.rs:1

but now this also went away. I will keep an eye out for further strange behavior. Thanks again! Will close it as the issue is fixed.

@Syphixs Syphixs closed this as completed Aug 22, 2024
@lulf
Copy link
Member

lulf commented Aug 22, 2024

Ah yes, that unsupported thing is iPhone specific. Basically iOS is probing using an illegal l2cap channel id, maybe for some Apple specific feature.

Should probably make that less error-y

@HaoboGu
Copy link

HaoboGu commented Sep 13, 2024

@lulf I got the same unsupported l2cap channel id 58 error on both iOS and macOS. Then iOS and macOS would disconnect. It there any workaround for it right now?

my trace:

INFO  Connected
└─ rmk::ble::trouble::run_ble_task::{async_fn#0}::{async_block#1} @ /Users/haobogu/Projects/keyboard/rmk/rmk/src/ble/trouble.rs:269 
WARN  [host] unsupported l2cap channel id 58
└─ trouble_host::host::{impl#2}::handle_acl @ /Users/haobogu/.cargo/git/checkouts/trouble-148cf413cadab653/d9d2e80/host/src/fmt.rs:156 
WARN  [host] encountered error processing ACL data for ConnHandle(0): NotSupported
└─ trouble_host::host::{impl#2}::run_with_handler::{async_fn#0}::{async_block#2} @ /Users/haobogu/.cargo/git/checkouts/trouble-148cf413cadab653/d9d2e80/host/src/fmt.rs:156 
TRACE [host] agreed att MTU of 185
└─ trouble_host::host::{impl#2}::handle_acl @ /Users/haobogu/.cargo/git/checkouts/trouble-148cf413cadab653/d9d2e80/host/src/fmt.rs:117 
INFO  GATT next event
└─ rmk::ble::trouble::run_ble_task::{async_fn#0}::{async_block#0} @ /Users/haobogu/Projects/keyboard/rmk/rmk/src/ble/trouble.rs:221 
ERROR Error processing GATT events: NotFound
└─ rmk::ble::trouble::run_ble_task::{async_fn#0}::{async_block#0}::{closure#0} @ /Users/haobogu/Projects/keyboard/rmk/rmk/src/ble/trouble.rs:252 
INFO  [host] disconnection event on handle 0, reason: Remote User Terminated Connection
└─ trouble_host::host::{impl#2}::run_with_handler::{async_fn#0}::{async_block#2} @ /Users/haobogu/.cargo/git/checkouts/trouble-148cf413cadab653/d9d2e80/host/src/fmt.rs:143 

@lulf
Copy link
Member

lulf commented Sep 13, 2024

Are you sure they are related? All iOS devices probes that l2cap channel id, but it should not cause a disconnect, at least that does not happen for me (nor for production firmware running trouble).

Are you using one of the example applications?

@HaoboGu
Copy link

HaoboGu commented Sep 13, 2024

Are you using one of the example applications?

ah I'm running my own code using trouble, not the example. I got Error processing GATT events: NotFound after connected. Here is my implementation. I have no idea why my implementation is problematic...

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 a pull request may close this issue.

3 participants