8000 ports: change block dev read/write methods to return integer errno instead of bool by dpgeorge · Pull Request #16223 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

ports: change block dev read/write methods to return integer errno instead of bool #16223

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

ports: change block dev read/write methods to return integer errno instead of bool #16223

wants to merge 5 commits into from
+76 −64

Conversation

dpgeorge
Copy link
Member

Summary

This is an alternative to #16222, and a more comprehensive fix. It changes all Python-level block dev functions so they return an integer errno code instead of a bool.

Fixes issues #16171 and #16214.

Testing

Tested on PYBV11 using:

import pyb, vfs
sdcard = pyb.SDCard()
vfs.VfsFat(sdcard)

That would previously fail, but with this patch passes.

TODO: test on esp32.

Trade-offs and Alternatives

This changes the API (the return value) of the stm32 and esp32 SDCard readblocks and writeblocks methods. A non-API-changing fix is in #16222.

I propose to merge #16222 to fix the issue for a patch release, then this PR to properly fix things (and eventually revert #16222).

@dpgeorge
extmod/vfs_blockdev: Provide and use func typedef for native read/write.
c7a9db1 
The readblocks/writeblocks functions, both native and Python versions, must
return 0 on success and negative errno integer on error.

These native typedefs help to document that.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
cc3200: Change sflash_disk functions to return int.
21843c8 
These are only called by the vfs_blockdev implementation, so their return
value doesn't need to be a FATFS specific type.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
stm32/sdcard: Change type of sdcard_read/write_blocks to return int.
59ed89a 
And make that 0 for success, negative errno integer for error.  And as a
consequence of this change the return value of the SDCard block device
Python bindings so they return an integer instead of a bool.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
stm32/storage: Change internal mp_uint_t to mp_int_t to match intent.
6f02d3d 
Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
esp32/machine_sdcard: Change SDCard read/write-blocks to return int.
bbc1f04 
This is what `vfs_blockdev.c` now expects, 0 for success and a negative
errno integer for failure.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge added the ports Relates to multiple ports, or a new/proposed port label Nov 13, 2024
This was referenced Nov 13, 2024
Merged
Closed
Closed
@codecov Codecov
Copy link
codecov bot commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.57%. Comparing base (611d8f9) to head (bbc1f04).

Files with missing lines Patch % Lines
extmod/vfs_blockdev.c 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #16223   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files         164      164           
  Lines       21345    21345           
=======================================
  Hits        21041    21041           
  Misses        304      304

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions GitHub Actions
Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:   -24 -0.003% standard
      stm32:    +8 +0.002% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

projectgus
projectgus reviewed Nov 15, 2024
View reviewed changes
Copy link
Contributor
@projectgus projectgus left a comment

Choose a reason for hiding this comment

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

This approach looks good to me for the long term. Let me know if I can help test on ESP32.

@dpgeorge dpgeorge mentioned this pull request Feb 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@projectgus projectgus projectgus left review comments

Assignees
No one assigned
Labels
ports Relates to multiple ports, or a new/proposed port
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dpgeorge @projectgus
0