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.

Sign up for GitHub

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

examples: add channelscan example that shows use of goroutines and channels #23

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

examples: add channelscan example that shows use of goroutines and channels #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58d88cd
examples: add channelscan example that shows use of goroutines and ch…
deadprogram Sep 16, 2020
afe3dff
build: add channelscan example to smoketests
deadprogram Sep 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ smoketest-tinygo:
@md5sum test.hex
$(TINYGO) build -o test.uf2 -size=short -target=circuitplay-bluefruit ./examples/discover
@md5sum test.hex
$(TINYGO) build -o test.uf2 -size=short -target=circuitplay-bluefruit ./examples/channelscan
@md5sum test.hex
$(TINYGO) build -o test.hex -size=short -target=pca10040-s132v6 ./examples/heartrate
@md5sum test.hex
$(TINYGO) build -o test.hex -size=short -target=reelboard-s140v7 ./examples/ledcolor
Expand Down Expand Up @@ -41,19 +43,22 @@ smoketest-linux:
GOOS=linux go build -o /tmp/go-build-discard ./examples/nusserver
GOOS=linux go build -o /tmp/go-build-discard ./examples/scanner
GOOS=linux go build -o /tmp/go-build-discard ./examples/discover
GOOS=linux go build -o /tmp/go-build-discard ./examples/channelscan

smoketest-windows:
# Test on Windows.
GOOS=windows CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc go build -o /tmp/go-build-discard ./examples/scanner
GOOS=windows CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc go build -o /tmp/go-build-discard ./examples/discover
GOOS=windows CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc go build -o /tmp/go-build-discard ./examples/heartrate-monitor
GOOS=windows CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc go build -o /tmp/go-build-discard ./examples/channelscan

smoketest-macos:
# Test on macos.
GOOS=darwin CGO_ENABLED=1 go build -o /tmp/go-build-discard ./examples/scanner
GOOS=darwin CGO_ENABLED=1 go build -o /tmp/go-build-discard ./examples/discover
GOOS=darwin CGO_ENABLED=1 go build -o /tmp/go-build-discard ./examples/nusclient
GOOS=darwin CGO_ENABLED=1 go build -o /tmp/go-build-discard ./examples/heartrate-monitor
GOOS=darwin CGO_ENABLED=1 go build -o /tmp/go-build-discard ./examples/channelscan

gen-uuids:
# generate the standard service and characteristic UUIDs
Expand Down
71 changes: 71 additions & 0 deletions examples/channelscan/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// This example program shows using Go routines and channels to coordinate
// BLE scanning.
//
// The first Go routine starts scanning using the BLE adaptor. When it finds
// a new device, it puts the information into a channel so it can be displayed.
//
// The second Go routine is a ticker that puts a "true" value into a channel every 3 seconds.
//
// The main function uses a select{} statement to wait until one of the two channels is unblocked
// by receiving data. If a new device is found, the boolean variable named "found" will
// be set to true, so that the timeout is reset for each 3 second period.
package main

import (
"time"

"tinygo.org/x/bluetooth"
)

var (
adapter = bluetooth.DefaultAdapter
devices = make(chan bluetooth.ScanResult, 1)
ticker = make(chan bool, 1)
found = true
)

func main() {
// Enable BLE interface.
if err := adapter.Enable(); err != nil {
panic("failed to enable adaptor:" + err.Error())
}

// Start scanning
go performScan()

// Start timeout ticker
go startTicker()

// Wait for devices to be scanned
for {
select {
case device := <-devices:
found = true
println("found device:", device.Address.String(), device.RSSI, device.LocalName())
case <-ticker:
if !found {
println("no devices found in last 3 seconds...")
}
found = false
}
}
}

func performScan() {
println("scanning...")

err := adapter.Scan(func(adapter *bluetooth.Adapter, device bluetooth.ScanResult) {
devices <- device
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is actually still a short race condition here.
Basically, the scan result will get overwritten once the next scan result comes in. So if the main goroutine isn't fast enough to process this scan result, it might display some duplicate scan results (and miss one of them).

I'm not sure how (or if) this should be fixed. One possibly expensive way is to stop scanning on each incoming packet, and start it again after the result is displayed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One solution would be to create local struct with fields we need from bluetooth.ScanResult and make the channel of that type

Suggested change
devices <- device
devices <- scanData{
name: device.LocalName(),
address: device.Address.String(),
rssi: device.RSSI,
}

Copy link
Member

@aykevl aykevl Sep 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Both device.LocalName() and device.Address.String() do heap allocations which are not allowed inside interrupts. If heap allocations were allowed, that would indeed be the most sensible solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, looking again at the code, the issue is bigger: devices <- device is a blocking send. This will usually work if the main goroutine is fast enough, but there is no guarantee that it will be. It would be better to have a non-blocking send like this:

select {
case devices <- device:
default:
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aykevl I actually ran my suggestion on NRF52 board and didn't get panic even if -print-allocs shows heap allocation. Looking at the Scan implementation for NRF52, the provided callback is called directly (from the context of Scan) and not from interrupt handler. Also there is a comment that scan is stopped when first result comes and a call to restart the scan every time the result comes. If this is correct (I would have to check SD documentation) it would mean that on NRF52 device there would be no race (with my suggestion) and also that blocking the Scan callback would not be an issue since scan is stopped during it's execution and restarted after (which explains why I get so many duplicate results in the scan). I didn't look at implementations for other targets but I hope the scan callback is also decoupled there because it is much more convenient that way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HattoriHanzo031 I checked the code and you are correct. The code in the PR seems fine.

})
if err != nil {
panic("failed to scan:" + err.Error())
}
}

func startTicker() {
for {
time.Sleep(3 * time.Second)
ticker <- true
}

}
Comment on lines +65 to +71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think nowadays you can also use time.NewTicker for this.