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.

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

Implemented Addresser setters have no effect #144

Closed
TirelessDev opened this issue Feb 16, 2023 · 3 comments
Closed

Implemented Addresser setters have no effect #144

TirelessDev opened this issue Feb 16, 2023 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@TirelessDev
Copy link
Contributor

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.

bluetooth/gap_darwin.go

Lines 27 to 33 in e79ea1e

func (ad Address) Set(val string) {
uuid, err := ParseUUID(val)
if err != nil {
return
}
ad.UUID = uuid
}

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
Copy link
Member

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

@aykevl aykevl self-assigned this Apr 27, 2023
aykevl added a commit that referenced this issue Apr 27, 2023
Without it, these calls are a no-op.

Fixes: #144

In particular, this fixes a problem where IsRandom() would always return
false on Linux. With this fix, it correctly returns whether the address
is a random address.
@aykevl
Copy link
Member

aykevl commented Apr 27, 2023

This is indeed a bug, thanks for spotting it!
Here is a fix: #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.

@deadprogram deadprogram added bug Something isn't working next-release labels Apr 29, 2023
@TirelessDev
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants