Skip to content

Conversation

@bznein
Copy link
Collaborator

@bznein bznein commented Sep 18, 2025

This way, the Bitbox backend, when running in simulator mode, can query for the simulated device's version and product.

@bznein bznein requested a review from benma September 18, 2025 10:01

// Info exposes the underlying info call, given a Communication.
func Info(communication Communication) (*semver.SemVer, common.Product, bool, error) {
device := NewDevice(nil, nil, nil, communication, nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to move the impl of info() here, and call Info(device.communication) inside device.info()? calling NewDevice() is overkill, and could lead to bad effects if we change what NewDevice does in the future (e.g. like the idea we discussed to have it call inferVersionAndProduct() right away, which would not be good here :P)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better idea, done :)

Comment on lines 228 to 229
// info uses the opInfo api endpoint to learn about the version, platform/edition, and unlock
// status (true if unlocked).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this docstring to Info(), the other docstring is not out of date.

This way, the Bitbox backend, when running in simulator mode, can query
for the simulated device's version and product.
Copy link
Contributor

@benma benma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@bznein bznein merged commit ba77092 into BitBoxSwiss:master Sep 18, 2025
3 checks passed
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