-
Notifications
You must be signed in to change notification settings - Fork 110
maintenance: make BBApp able to talk to simulator. #3560
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
Conversation
e20717b
to
7e99207
Compare
832dfe8
to
63a23ae
Compare
@benma this is ready for another review. The only part I haven't addresses (but I left a comment) is the "IsRunning" method, which I agree is not ideal, but I can't find a different way. |
63a23ae
to
4e48f6d
Compare
b69361b
to
8b750b7
Compare
product == bitbox02common.ProductBitBox02PlusMulti { | ||
if product != nil && (*product == bitbox02common.ProductBitBox02PlusBTCOnly || | ||
*product == bitbox02common.ProductBitBox02PlusMulti) { | ||
productName = BitBox02NovaProductName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also has to happen if the product name is inferred in instead of passed directly
backend/devices/bitbox02/device.go
Outdated
WithField("productName", productName) | ||
|
||
if product != nil { | ||
log = log.WithField("product", *product) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, would be nice to log it also if it was inferred vs passed
|
||
// Product implements usb.DeviceInfo. | ||
func (d deviceInfo) Product() string { | ||
return bitbox02common.FirmwareDeviceProductStringBitBox02Multi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would not need that anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK, this will be super useful, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually forgot there were some comments left to address 😇
3e31227
to
569a378
Compare
@benma ready again :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
|
||
// Serial implements usb.DeviceInfo. | ||
func (d deviceInfo) Serial() string { | ||
return fmt.Sprintf("v%s", d.version.String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fyi, .String()
is technically not needed, as it is called when using %s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently this causes the tests to fail: https://github.com/BitBoxSwiss/bitbox-wallet-app/actions/runs/17909896855/job/50918793278?pr=3560
569a378
to
6865016
Compare
This allows devs to use the app without a real device and without relying on the software keystore.
6865016
to
c1d4ce2
Compare
Right now, this is only enabled for the servewallet, as it is intended to be used in automated testing that uses it.
Before asking for reviews, here is a check list of the most common things you might need to consider: