From 63296fbafbd7930616f9435b6a2de1cbfc845936 Mon Sep 17 00:00:00 2001
From: Amr Bekhit <amrbekhit@gmail.com>
Date: Thu, 23 Mar 2023 14:46:37 +0300
Subject: [PATCH] Make reporting of parse result non-blocking in
 WatchDeviceChanges

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:

```go
	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:

```go
    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:

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

    // No need to empty the channel now
```
---
 api/beacon/beacon.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/api/beacon/beacon.go b/api/beacon/beacon.go
index fc49863..1b9076f 100644
--- a/api/beacon/beacon.go
+++ b/api/beacon/beacon.go
@@ -68,7 +68,10 @@ func (b *Beacon) WatchDeviceChanges(ctx context.Context) (chan bool, error) {
 				}
 
 				if changed.Name == "ManufacturerData" || changed.Name == "ServiceData" {
-					ch <- b.Parse()
+					select {
+					case ch <- b.Parse():
+					default:
+					}
 				}
 
 				break