Skip to content

Allow DeviceInfo to use native Go types #63

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

daniel-sullivan
Copy link

Inspired by #30

This originally come from the requirement to have a consistent device ID which can be stored as a string (for example in a UI select box) and then be used to select the same device later.

@gen2brain
Copy link
Owner

Thanks. I don't like including C and header file just for a few constants, is it really needed?

@daniel-sullivan
Copy link
Author

Completely agree! Unfortunately, not including it results in:

../../constants.go:10:10: could not determine what C.ma_bool32 refers to

unless the header is included in the same file.

I tried to maintain the existing format (with constants being in constants.go)

There's a couple of alternates if you prefer:

  • I could move the ma_true and ma_false definitions into device_info.go where the header's already included
  • Change the line newDevice.IsDefault = device.isDefault == True to newDevice.IsDefault = device.isDefault == C.ma_true

@gen2brain
Copy link
Owner

Is ma_bool32 really needed? These are just 1 and 0.

@daniel-sullivan
Copy link
Author

daniel-sullivan commented Mar 25, 2025

Not really. If you're happy hardcoding to those, that makes everything a lot simpler. I've just had upstream libraries I'm wrapping screw around with values you expect to be constant so I'm fairly defensive.

@gen2brain
Copy link
Owner

Yes please, that will never change I guess, so hardcoding is fine.

@daniel-sullivan
Copy link
Author

Just for cross-linking, this PR should also fix #28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants