Skip to content
Snippets Groups Projects

make import work in micropython

Merged swym requested to merge swym/firmware:import into master
8 unresolved threads

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 49 [FR_TOO_MANY_OPEN_FILES] = EMFILE,
    50 [FR_INVALID_PARAMETER] = EINVAL,
    51 };
    52
    53 enum FatObjectType { FO_Nil, FO_File, FO_Dir };
    54 struct FatObject {
    55 enum FatObjectType type;
    56 union {
    57 FIL file;
    58 DIR dir;
    59 };
    60 };
    18 61
    19 62 static bool mount(void);
    63 static int
    64 get_fat_object(int i, enum FatObjectType expected, struct FatObject **res);
    • I'm wondering if it a good idea to have i being equivalent to fd. We might catch more dangling descriptors if we make fd part of FatObject and iterate over it. Just a thought though.

    • Author Maintainer

      I've been thinking about this. I quite enjoy the fact that the fd is also the index into our array. How about maintaining a generation count of sorts in parallel and encoding the fd as 0xGGGGGGii - generation count in the highest 24bits and the array-index in the lowest 8bits. For validating the passed-in fd, the generation counter would then also be stored in struct FatObject. Starting this generation counter at 1 instead of 0 would also enable us to differentiate fat-FDs from stdin/stdout/stderr if we wanted to in the future.

    • Please register or sign in to reply
  • rahix
  • 221 }
    222 o = &s_openedObjects[i];
    223
    224 while (*mode_s) {
    225 switch (*mode_s++) {
    226 case 'r':
    227 mode |= FA_READ;
    228 break;
    229 case 'w':
    230 mode |= FA_WRITE | FA_CREATE_ALWAYS;
    231 break;
    232 case 'x':
    233 mode |= FA_WRITE | FA_CREATE_NEW;
    234 break;
    235 case 'a':
    236 mode |= FA_WRITE | FA_OPEN_ALWAYS;
  • 236 mode |= FA_WRITE | FA_OPEN_ALWAYS;
    237 break;
    238 case '+':
    239 mode |= FA_READ | FA_WRITE;
    240 break;
    241 }
    242 }
    243
    244 int res = f_open(&o->file, filename, mode);
    245 if (res != FR_OK) {
    246 return -fresult_to_errno_table[res];
    247 }
    248 o->type = FO_File;
    249
    250 // for 'a' mode, we must begin at the end of the file
    251 if ((mode & FA_OPEN_ALWAYS) != 0) {
  • 163 case 't':
    164 type = &mp_type_fat_textio;
    165 break;
    166 }
    167 }
    168
    169 pyb_file_obj_t *o = m_new_obj_with_finaliser(pyb_file_obj_t);
    170 o->base.type = type;
    171
    172 const char *fname = mp_obj_str_get_str(args[0].u_obj);
    173 int res = epic_open(fname, modeString);
    174 if (res < 0) {
    175 m_del_obj(pyb_file_obj_t, o);
    176 mp_raise_OSError(-res);
    177 }
    178 o->fd = res;
  • rahix
  • 1 #include "epicardium.h"
    2
    3 #include <py/runtime.h>
    4 #include <py/reader.h>
    5 #include <py/lexer.h>
    6
    7 /** ported from picropython's posic implementation */
    8
    9 typedef struct _mp_reader_epicfat_t {
    10 bool close_fd;
    11 int fd;
    12 size_t len;
    13 size_t pos;
    14 byte buf[20];
  • rahix
    rahix @rahix started a thread on the diff
  • 454 *
    455 * Except for epic_open, which models C stdio's fopen function, close, read and write
    456 * model close (3), read (3) and write(3)
    457 * All file-related functions return >= 0 on success and -Exyz on failure, with error
    458 * codes from errno.h (EIO, EINVAL etc.)
    459 *
    460 */
    461 API(API_FILE_OPEN, int32_t epic_open(const char* filename, const char* modeString));
    462 API(API_FILE_CLOSE, int32_t epic_close(int32_t fd));
    463 API(API_FILE_READ, int32_t epic_read(int32_t fd, void* buf, uint32_t nbytes));
    464 API(API_FILE_WRITE, int32_t epic_write(int32_t fd, const void* buf, uint32_t nbytes));
    465 API(API_FILE_FLUSH, int32_t epic_flush(int32_t fd));
    466
    467 enum epic_stat_type {
    468 EPICSTAT_FILE,
    469 EPICSTAT_DIR,
    • Is there a way to list a directory? / Did the micropython codebase show any signs of other ports providing this functionality?

    • Author Maintainer

      I have not looked at other ports so far, I've been avoiding looking at directories so far. Will be necessary for sure sooner or later... I have yet to work out the whole set of FS features we want and need.

    • Please register or sign in to reply
  • rahix
  • rahix
    rahix @rahix started a thread on the diff
  • 30 #include "py/runtime.h"
    31 #include "py/builtin.h"
    32 #include "py/stream.h"
    33 #include "py/mperrno.h"
    34
    35 #include "epicardium.h"
    36
    37 extern const mp_obj_type_t mp_type_fat_textio;
    38 #if MICROPY_PY_IO_FILEIO
    39 extern const mp_obj_type_t mp_type_fat_fileio;
    40 #endif
    41
    42 typedef struct _pyb_file_obj_t {
    43 mp_obj_base_t base;
    44 int fd;
    45 } pyb_file_obj_t;
  • @swym, thank you so much for your work! @schneider and me decided we'll merge it as it is right now, so the feature is available as soon as possible. Any of the open comments can be dealt with after that.

    I'll do two things ontop of this MR:

    1. Fix a few style things
    2. Rename the API functions to epic_file_*

    If you want to work on the global lock this weekend, you should probably base that off of the then master branch so you won't have to deal with too many merge conflicts.

  • Author Maintainer

    Awesome! I'll address remaining topics separately then.

    Edited by swym
  • rahix
    rahix @rahix started a thread on the diff
  • 467 enum epic_stat_type {
    468 EPICSTAT_FILE,
    469 EPICSTAT_DIR,
    470 };
    471
    472 typedef struct epic_stat_t {
    473 enum epic_stat_type type;
    474 } epic_stat_t;
    475
    476 /**
    477 * stat path
    478 *
    479 * This does not follow posix convention, but rather takes
    480 * a path as parameter. This aligns more with libff's API and
    481 * also this has been implemented for python import support, which
    482 * passes the filename as well.
  • merged

  • Please register or sign in to reply
    Loading