SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
721 stars 168 forks source link

MicroSD.is_inserted as an unsettable attribute #446

Closed jdlcdl closed 1 year ago

jdlcdl commented 1 year ago

This pr resolves an error-prone is_enabled() method call by accessing that name as an attribute unintended reference to a class attribute instead of calling the name as a class method effectively enabling user to pull the microsd before startup and allowing seedsigner to learn the microsd inserted/removed state.

kdmukai commented 1 year ago

Since the "is_" pattern is so ingrained as a property, I think the way is_inserted() is defined will continue to be error-prone.

We'd be better off either renaming the classmethod to a slightly different name that is more intuitively method-y (but I can't think of any such name) OR, even better, change it to be a @property:

# microsd.py
    ...
    @property
    def is_inserted(self):
        ...

# microsd.py MicroSD.run()
Settings.handle_microsd_state_change(
    action=MicroSD.ACTION__INSERTED if self.is_inserted else MicroSD.ACTION__REMOVED
)
# toast.py RemoveSDCardToastManagerThread.should_keep_running()
MicroSD.get_instance().is_inserted
newtonick commented 1 year ago

ACK and Tested