examples: add channelscan example that shows use of goroutines and channels #23
Conversation
|
I haven't looked at the code yet, but can you rebase this PR? There is a conflict. |
deadprogram
force-pushed
the
channelscan-example
branch
from
84a96c5
to
f3862f6
Compare
|
Rebased. |
…annels Signed-off-by: deadprogram <[email protected]>
Signed-off-by: deadprogram <[email protected]>
deadprogram
force-pushed
the
channelscan-example
branch
from
f3862f6
to
afe3dff
Compare
|
Corrections made and ready for review again. |
| func startTicker() { | ||
| for { | ||
| time.Sleep(3 * time.Second) | ||
| ticker <- true | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
I think nowadays you can also use time.NewTicker for this.
| println("scanning...") | ||
|
|
||
| err := adapter.Scan(func(adapter *bluetooth.Adapter, device bluetooth.ScanResult) { | ||
| devices <- device |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
One solution would be to create local struct with fields we need from bluetooth.ScanResult and make the channel of that type
| devices <- device | |
| devices <- scanData{ | |
| name: device.LocalName(), | |
| address: device.Address.String(), | |
| rssi: device.RSSI, | |
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
}There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
@HattoriHanzo031 I checked the code and you are correct. The code in the PR seems fine.
| println("scanning...") | ||
|
|
||
| err := adapter.Scan(func(adapter *bluetooth.Adapter, device bluetooth.ScanResult) { | ||
| devices <- device |
There was a problem hiding this comment.
@HattoriHanzo031 I checked the code and you are correct. The code in the PR seems fine.
This PR adds the "channelscan" example that shows use of goroutines and channels with Go Bluetooth.