PaulStoffregen / USBHost_t36

USB Host Library for Teensy 3.6 and 4.0
171 stars 87 forks source link

Update USBHost_t36.h #93

Closed KurtE closed 2 years ago

KurtE commented 2 years ago

@PaulStoffregen @mjs513 @wwatson4506 -

I confirmed by building one of the storage Example sketches that the code to get/set File dates for files in USBDrive/Filesystem was not compiled into the programs and as such used the default implemention that did not get or set dates.

This was caused by while we were developing it it was under the

iffdef FS_FILE_SUPPORT_DATES

Which we add into FS.h.

It was later removed from FS.h

So I changed this test to check for version of Teensyduino.

if TEENSYDUINO >= 156

Hopefully we can get this in for the next Beta and/or Release

KurtE commented 2 years ago

@PaulStoffregen @mjs513 @wwatson4506 -

Warning all: I don't think the update is correct yet...

In particular I think it is using the old style Date/Time functions not the updated versions...

That is the current code (Note I only changed the #if ...

#if TEENSYDUINO >= 156  
    // These will all return false as only some FS support it.
    virtual bool getAccessDateTime(uint16_t* pdate, uint16_t* ptime) {
        return mscfatfile.getAccessDateTime(pdate, ptime);
    }
    virtual bool getCreateDateTime(uint16_t* pdate, uint16_t* ptime) {
        return mscfatfile.getCreateDateTime(pdate, ptime);
    }
    virtual bool getModifyDateTime(uint16_t* pdate, uint16_t* ptime) {
        return mscfatfile.getModifyDateTime(pdate, ptime);
    }
    virtual bool timestamp(uint8_t flags, uint16_t year, uint8_t month, uint8_t day,
               uint8_t hour, uint8_t minute, uint8_t second) {
        return mscfatfile.timestamp(flags, year, month, day, hour, minute, second);
        return false;
    }
#endif

Now if you look at the version in SD.h we have:

    virtual bool getCreateTime(DateTimeFields &tm) {
        uint16_t fat_date, fat_time;
        if (!sdfatfile.getCreateDateTime(&fat_date, &fat_time)) return false;
        if ((fat_date == 0) && (fat_time == 0)) return false;
        tm.sec = FS_SECOND(fat_time);
        tm.min = FS_MINUTE(fat_time);
        tm.hour = FS_HOUR(fat_time);
        tm.mday = FS_DAY(fat_date);
        tm.mon = FS_MONTH(fat_date) - 1;
        tm.year = FS_YEAR(fat_date) - 1900;
        return true;
    }
    virtual bool getModifyTime(DateTimeFields &tm) {
        uint16_t fat_date, fat_time;
        if (!sdfatfile.getModifyDateTime(&fat_date, &fat_time)) return false;
        if ((fat_date == 0) && (fat_time == 0)) return false;
        tm.sec = FS_SECOND(fat_time);
        tm.min = FS_MINUTE(fat_time);
        tm.hour = FS_HOUR(fat_time);
        tm.mday = FS_DAY(fat_date);
        tm.mon = FS_MONTH(fat_date) - 1;
        tm.year = FS_YEAR(fat_date) - 1900;
        return true;
    }
    virtual bool setCreateTime(const DateTimeFields &tm) {
        if (tm.year < 80 || tm.year > 207) return false;
        return sdfatfile.timestamp(T_CREATE, tm.year + 1900, tm.mon + 1,
            tm.mday, tm.hour, tm.min, tm.sec);
    }
    virtual bool setModifyTime(const DateTimeFields &tm) {
        if (tm.year < 80 || tm.year > 207) return false;
        return sdfatfile.timestamp(T_WRITE, tm.year + 1900, tm.mon + 1,
            tm.mday, tm.hour, tm.min, tm.sec);
    }

Which was updated to not have our interface use for SDFat Timestamp member as part of our interface.

I can update again which is fine.

But I keep wondering do we really need to duplicate this code?

That is if in SD.h where we have:

class SDFile : public FileImpl
{
private:
    // Classes derived from File are never meant to be constructed
    // anywhere other than open() in the parent FS class and
    // openNextFile() while traversing a directory.
    // Only the abstract File class which references these derived
    // classes is meant to have a public constructor!
    SDFile(const SDFAT_FILE &file) : sdfatfile(file), filename(nullptr) { }
    friend class SDClass;
public:
...

If the constructor was public is there any reason why it would not work for both places... 
Maybe could have different name, like FatFileWrapper  or the like.

Thoughts?

EDIT: I pushed up changes with copies of the function from SD.h (only diffferences) is the name of the member variable that holds a copy of the FsFile object