lancaster-university / codal-core

MIT License
12 stars 27 forks source link

Ghost FAT driver prevents compilation when USB is enabled #134

Open jamesadevine opened 3 years ago

jamesadevine commented 3 years ago

https://github.com/lancaster-university/codal-core/blob/c8547baf94649b20a80457fffb279078e9d32a0d/source/drivers/GhostFAT.cpp#L4

@mmoskal is there anyway we can be more precise with the conditional compilation of the GhostFAT driver?

The board I am working on has not defined BOOTLOADER_START_ADDR and UF2_DEFINE_HANDOVER so functions required by the GhostFAT driver are not being defined by uf2format.h. This breaks compilation:

/Users/James/Desktop/VM/MSR/makeable/libraries/codal-core/source/drivers/GhostFAT.cpp: In member function 'virtual void codal::GhostFAT::writeBlocks(int, int)':
/Users/James/Desktop/VM/MSR/makeable/libraries/codal-core/source/drivers/GhostFAT.cpp:304:22: error: 'uf2_info' was not declared in this scope
  304 |     const char *p0 = uf2_info(), *p = p0;
      |                      ^~~~~~~~
/Users/James/Desktop/VM/MSR/makeable/libraries/codal-core/source/drivers/GhostFAT.cpp:305:13: error: 'p' was not declared in this scope; did you mean 'p0'?
  305 |     while (*p && *p != '\n')
      |             ^
      |             p0
/Users/James/Desktop/VM/MSR/makeable/libraries/codal-core/source/drivers/GhostFAT.cpp:307:12: error: 'p' was not declared in this scope; did you mean 'p0'?
  307 |     while (p > p0)
      |            ^
      |            p0
/Users/James/Desktop/VM/MSR/makeable/libraries/codal-core/source/drivers/GhostFAT.cpp:324:17: error: 'check_uf2_handover' was not declared in this scope
  324 |                 check_uf2_handover(buf, numBlocks, in->ep & 0xf, out->ep & 0xf, cbwTag());
      |                 ^~~~~~~~~~~~~~~~~~
/Users/James/Desktop/VM/MSR/makeable/libraries/codal-core/source/drivers/GhostFAT.cpp: In member function 'virtual void codal::GhostFAT::addFiles()':
/Users/James/Desktop/VM/MSR/makeable/libraries/codal-core/source/drivers/GhostFAT.cpp:470:19: error: 'uf2_info' was not declared in this scope
  470 |     addStringFile(uf2_info(), "info_uf2.txt");
      |                   ^~~~~~~~

Do you see any issue with guarding the compilation of this driver using the BOOTLOADER_START_ADDR define?

jamesadevine commented 3 years ago

@mmoskal thoughts?

mmoskal commented 3 years ago

yes, you can disable the whole driver, but what prevents you from specifying the bootloader addresses?

jamesadevine commented 3 years ago

Nothing on my particular board. Though, in the future we might have some boards that support USB, but do not yet have UF2. I think it is better to not cause a compiler error by default in any case.

mmoskal commented 3 years ago

so if bootloader range is not defined you could just not add "info_uf2.txt" file and don't check for handover in writeBlocks. Typically the user would instantiate (or derive from) this class and add more files.

jamesadevine commented 3 years ago

@mmoskal see https://github.com/lancaster-university/codal-core/pull/135 . Let me know if this patch looks good.