muka / go-bluetooth

Golang bluetooth client based on bluez DBus interfaces
Apache License 2.0
653 stars 123 forks source link

Make reporting of parse result non-blocking in WatchDeviceChanges #185

Open amrbekhit opened 1 year ago

amrbekhit commented 1 year ago

The WatchDeviceChanges function creates a goroutine that watches for any change notifications coming from the underlying Device. The result of b.Parse is sent to the channel ch using a blocking send. Because of this, the caller to WatchDeviceChanges must continuously empty this channel otherise the goroutine will simply hang on this line, meaning it will never exit and result in a goroutine leak. So the caller code looks something like this:

ch, err := b.WatchDeviceChanges(ctx)
if err != nil {
      ...
}

// Make sure to empty the channel
go func() {
    for range ch {}
}()

The use of ch to return the value of b.Parse here seems a bit clunky. It would be useful to just call WatchDeviceChanges and check for errors without needing to do any other maintenance except call UnwatchDeviceChanges when the beacon is removed. Is ch used to notify the caller that the beacon properties have changed so that they can take action? If so, is it critical that no events are missed such that a blocking send is required? If not, perhaps the blocking send to ch in the goroutine could be changed to a non-blocking one. This simplifies the caller code, as they can choose to ignore the channel if they don't need it:

if changed.Name == "ManufacturerData" || changed.Name == "ServiceData" {
    select {
        case ch <- b.Parse():
            // Result of parse sent to channel
        default:
            // The channel is blocked, so result is discarded
    }
}

The caller code is thus simplified:

_, err := b.WatchDeviceChanges(ctx)
if err != nil {
    ...
}

// No need to empty the channel now