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

v2: use a byte array for the characteristics bitfield #240

Open
mvdan opened this issue Sep 20, 2021 · 1 comment
Open

v2: use a byte array for the characteristics bitfield #240

mvdan opened this issue Sep 20, 2021 · 1 comment
Labels
P3 Low: Not priority right now

Comments

@mvdan
Copy link
Contributor

mvdan commented Sep 20, 2021

Right now, characteristics is as follows:

type Characteristics struct {
	Hi uint64
	Lo uint64
}

And, as per the code, it encodes and decodes as little-endian.

If Go had a uint128 type, I'd definitely agree with this approach. For better or worse it doesn't, so using encoding/binary.LittleEndian doesn't save us complexity - we still have to choose between Hi and Lo.

I think it would be easier to use [16]byte, as proposed by @rvagg here: #239 (comment)

Then the logic should also become fairly straightforward to follow. For example, a "get bit" or "set bit" method on Characteristics would use pos/8 to select an array index, and pos%8 to select the bit within its byte.

@mvdan
Copy link
Contributor Author

mvdan commented Sep 20, 2021

Another note: we could also define nice user-friendly APIs, such as:

type Characteristic uint8

const FooBar Characteristic = 1
// ...

func (Characteristics) Get(c Characteristic) bool { ... }
func (Characteristics) Set(c Characteristic, enabled bool) { ... }

This sorta assumes that all characteristics will be booleans, but I think that's the idea.

@BigLep BigLep added the P3 Low: Not priority right now label May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Low: Not priority right now
Projects
None yet
Development

No branches or pull requests

2 participants