sdfat - correct logic of datalog

i wish to write a datalog, from the example of the author we could see this datalog which is very near to what i need

i have a question about these lines:

if (!file.sync() || file.getWriteError()) {
    error("write error");
}

if i am not wrong sync() should return 1 if there is no error, infact it has the "!" before.
getWriteError() should return 1 if there is no problem, why is it putted there? in this way the code inside the if will be executed! maybe there is a "!" missing or there are other reason?

since flush() calls sync() isn't better to call it direcly and forget about flush()?

in the case the author will see this: which is the best way up to now to write a datalog which have error handling like this example?

aster94:
getWriteError() should return 1 if there is no problem

Where did you find that information?

https://github.com/greiman/SdFat-beta/blob/master/SdFat/src/FatLib/ArduinoFiles.h

line 87

I don't think you're drawing the correct conclusion from that comment. The standard for Arduino functions named write is that they should return the number of bytes written so that's the reason why 0 is returned when the write fails but you shouldn't consider that 0 an error code, it just means 0 bytes were written successfully. getWriteError() will give you the actual error code to explain the cause of the write failure. I'm not very familiar with this library, haven't made much of an effort to read through the source, and the documentation is not very good but I did find a list of error codes (though I'm only guessing they apply to getWriteError()):
https://github.com/greiman/SdFat-beta/blob/master/SdFat/src/SdCard/SdInfo.h#L37-L86

// SD card errors
// See the SD Specification for command info.
typedef enum {
  SD_CARD_ERROR_NONE = 0,

  // Basic commands and switch command.
  SD_CARD_ERROR_CMD0 = 0X20,
  SD_CARD_ERROR_CMD2,
  SD_CARD_ERROR_CMD3,
  SD_CARD_ERROR_CMD6,
  SD_CARD_ERROR_CMD7,
  SD_CARD_ERROR_CMD8,
  SD_CARD_ERROR_CMD9,
  SD_CARD_ERROR_CMD10,
  SD_CARD_ERROR_CMD12,
  SD_CARD_ERROR_CMD13,

  // Read, write, erase, and extension commands.
  SD_CARD_ERROR_CMD17 = 0X30,
  SD_CARD_ERROR_CMD18,
  SD_CARD_ERROR_CMD24,
  SD_CARD_ERROR_CMD25,
  SD_CARD_ERROR_CMD32,
  SD_CARD_ERROR_CMD33,
  SD_CARD_ERROR_CMD38,
  SD_CARD_ERROR_CMD58,
  SD_CARD_ERROR_CMD59,

  // Application specific commands.
  SD_CARD_ERROR_ACMD6 = 0X40,
  SD_CARD_ERROR_ACMD13,
  SD_CARD_ERROR_ACMD23,
  SD_CARD_ERROR_ACMD41,

  // Read/write errors
  SD_CARD_ERROR_READ = 0X50,
  SD_CARD_ERROR_READ_FIFO,
  SD_CARD_ERROR_READ_REG,
  SD_CARD_ERROR_READ_START,
  SD_CARD_ERROR_READ_TIMEOUT,
  SD_CARD_ERROR_STOP_TRAN,
  SD_CARD_ERROR_WRITE,
  SD_CARD_ERROR_WRITE_FIFO,
  SD_CARD_ERROR_WRITE_START,
  SD_CARD_ERROR_WRITE_TIMEOUT,

    // Misc errors.
  SD_CARD_ERROR_DMA = 0X60,
  SD_CARD_ERROR_ERASE,
  SD_CARD_ERROR_ERASE_SINGLE_BLOCK,
  SD_CARD_ERROR_ERASE_TIMEOUT,
  SD_CARD_ERROR_INIT_NOT_CALLED,
  SD_CARD_ERROR_FUNCTION_NOT_SUPPORTED
} sd_error_code_t;

Note that SD_CARD_ERROR_NONE is 0.

Pert you are right

return 1 for success and 0 for failure.

I thought that this was referred to getwriteerror() but as you said is referred to write()
I would need to find that function but right now i am from my mobile phone and it is not easy

About that error list, i saw it but i think it is quite uncompleted, maybe he didn t implement it yet... Idk...

It sounds like those errors are part of the SD specification so it may be they just created an enum for every possible one but haven't actually implemented all of them in the library.

well let's see if greiman cames here and could give some help

anyway here there is getWriteError

bool getWriteError() {
    return m_error & WRITE_ERROR;
  }

it is strange since m_error should already include WRITE_ERROR

bool FatFile::sync() {
  if (!isOpen()) {
    return true;
  }
  if (m_flags & F_FILE_DIR_DIRTY) {
    dir_t* dir = cacheDirEntry(FatCache::CACHE_FOR_WRITE);
    // check for deleted by another open file object
    if (!dir || dir->name[0] == DIR_NAME_DELETED) {
      DBG_FAIL_MACRO;
      goto fail;
    }
    // do not set filesize for dir files
    if (isFile()) {
      dir->fileSize = m_fileSize;
    }

    // update first cluster fields
    dir->firstClusterLow = m_firstCluster & 0XFFFF;
    dir->firstClusterHigh = m_firstCluster >> 16;

    // set modify time if user supplied a callback date/time function
    if (m_dateTime) {
      m_dateTime(&dir->lastWriteDate, &dir->lastWriteTime);
      dir->lastAccessDate = dir->lastWriteDate;
    }
    // clear directory dirty
    m_flags &= ~F_FILE_DIR_DIRTY;
  }
  if (m_vol->cacheSync()) {
    return true;
  }
  DBG_FAIL_MACRO;

fail:
  m_error |= WRITE_ERROR;
  return false;
}

same on fatFile::write()

so, if someone wants to know if there is ay problem, maybe it is enough to call

uint8_t getError() {
    return m_error;
  }

maybe he doesn't know it so i opened an issue on github
I also found this in changes.txt:
Override Print::getWriteError() and Print::clearWriteError() to use
SdBaseFile functions.

should it mean that these function are here only for compatibility?