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

minor patches #9

Merged
merged 4 commits into from
Nov 16, 2023
Merged

minor patches #9

merged 4 commits into from
Nov 16, 2023

Conversation

hstarmans
Copy link

@hstarmans hstarmans commented Nov 15, 2023

as outlined in #8

Thanks for this great library! Helped me a lot.
I think the following could be done to make it even better.
I already added my suggested improvements.

change await function

# last bit (1) is BUSY bit in stat. reg. byte (0 = not busy, 1 = busy)
trials = 0
while 0x1 & self.spi.read(1, 0xFF)[0]:
    trials += 1
    if trials > 20:
         raise Exception("Can't read from device")
    sleep(0.1)

Current code can lead to infinite loop and you should do somethingl like above

writeblocks should not check block length
You check block length and raise warning if not satisfied.
This is against spec (https://docs.micropython.org/en/latest/library/os.html).
Also gives issue if you write a file raw to memory with a size not equal to multiple of blocks

if len(buf) %buffsize != 0:
    buf += bytearray((buffsize-(len(buf)*[255]))
# NOTE: in my final fix I remove the appended bytes

don't break just raise warning
I would do something like this in the identify function.

if mf != 0xEF or mem_type not in [0x40, 0x60, 0x70]:
    # Winbond manufacturer, Q25 series memory (tested 0x40 only)
    print(f"manufacturer ({}) or memory type ({}) note tested")

@brainelectronics
Copy link
Owner

Thank you for your contribution @hstarmans

I haven't yet updated this package with the latest contribution requirements/guidelines as done here in the package template

May you could update the changelog.md and the winbond/version.py file?

@hstarmans
Copy link
Author

Added changes as requested!

Copy link
Owner

@brainelectronics brainelectronics left a comment

Choose a reason for hiding this comment

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

@hstarmans
Copy link
Author

Sure, I added a config for pre-commit so a check is carried by git before a push is made to the repo. This is also recommended by micropython. The config is minimal. For a more extended example see https://github.com/sktime/sktime/blob/main/.pre-commit-config.yaml .

@brainelectronics brainelectronics merged commit 1838d2e into brainelectronics:main Nov 16, 2023
2 of 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