search

tinygo-org / bluetooth

Cross-platform Bluetooth API for Go and TinyGo. Supports Linux, macOS, Windows, and bare metal using Nordic SoftDevice or HCI
https://tinygo.org
Other
708 stars 134 forks source link

Implemented Addresser setters have no effect #144

Closed TirelessDev closed 1 year ago

TirelessDev commented 1 year ago

The Addresser interface declares the Set(val string) and SetRandom(bool) receivers but all implementations implement these as value receivers that ineffectively attempt to modify their own state. This is triggering the go static check SA4005.

https://github.com/tinygo-org/bluetooth/blob/e79ea1e4e99c2628e7219de0bd88da59adaf3d60/gap_darwin.go#L27-L33

These implementations should be pointer receivers but changing them will unfortunately modify the API. All Address objects would need to be passed around as *Address pointers as the Address type will no longer implement the Addresser interface. This is most obvious in func (a *Adapter) Connect(address Addresser, params ConnectionParams) (*Device, error).

deadprogram commented 1 year ago

See the recent updates to dev which change this in order to avoid heap allocations.

aykevl commented 1 year ago

This is indeed a bug, thanks for spotting it! Here is a fix: https://github.com/tinygo-org/bluetooth/pull/158

These implementations should be pointer receivers but changing them will unfortunately modify the API. All Address objects would need to be passed around as *Address pointers as the Address type will no longer implement the Addresser interface.

At least with the current dev branch, no API change is necessary.

TirelessDev commented 1 year ago

Awesome! Thanks all, the changes look good. I have also updated PR #149 to reflect the latest dev.