embassy-rs / trouble

A Rust Host BLE stack with a future goal of qualification.
Apache License 2.0
87 stars 11 forks source link

`Advertiser::drop` always assumes extended advertising #66

Closed Univa closed 2 months ago

Univa commented 2 months ago

The Drop implementation for Advertiser calls self.advertise_command_state.cancel(true). The true boolean value corresponds to extended advertising, based on the run_with_handler implementation.

In scenarios where you are not using extended advertising, the host could send an LeSetExtAdvEnable (with a false value) command, if the Advertiser struct gets dropped.

For example, this can cause issues with the apache-nimble controller:

let conn = adapter
    .advertise(
        &Default::default(),
        Advertisement::ConnectableScannableUndirected {
            adv_data: &adv_data,
            scan_data: &[],
        },
    )
    .await
    .unwrap()
    .accept()
    .await
    .unwrap();

Since the code above will call Advertiser::drop, the host will send the LeSetExtAdvEnable command, then the apache-nimble controller would return a "Command Disallowed" error.

lulf commented 2 months ago

Thanks for the report. This is a regression after I introduced the advertiser. It should be changed to know which kind of advertiser it is and pass that on when signalling the cancel. Either a bool param, or a generic param on the Advertiser, I think I prefer the bool param.