No SD_CHECK_AND_RETRY with USE_USB_COMPOSITE (STM32F103 + SDIO)

[Bob-the-Kuhn]

SD_CHECK_AND_RETRY is not compatible with USE_USB_COMPOSITE. The resulting virtual drive causes error popups on the PC and is eventually disabled by the PC.

This fixes Issue #18097.

The proposed change is to append !ENABLED(USE_USB_COMPOSITE) to #if ENABLED(SD_CHECK_AND_RETRY) in Sd2Card.cpp.

The only other effective change I found was adding a check to SanityCheck.h.

/**
 * STM32F1xx boards with SDIO have run time errors if SD_CHECK_AND_RETRY and USE_USB_COMPOSITE are both enabled.
 */
#if ALL(SDSUPPORT, SD_CHECK_AND_RETRY, USE_USB_COMPOSITE, __STM32F1__ ) && SD_CONNECTION_IS(ONBOARD) && ANY(STM32_HIGH_DENSITY, STM32_XL_DENSITY)
  #error "Disable SD_CHECK_AND_RETRY. Retries and CRC automatically implemented.
#18018 

Adding a #undef SD_CHECK_AND_RETRY to the pins_YOUR_BOARD.h files or to sdio.cpp did not work.

---

USE_USB_COMPOSITE is only available in the following environments:

• STM32F103RC_btt_USB
• STM32F103RC_btt_512K_USB
• STM32F103RE_btt_USB

These are used only with the following boards:

• BTT_SKR_MINI_V1_1
• BTT_SKR_MINI_E3_V1_0
• BTT_SKR_MINI_E3_V1_2
• BTT_SKR_E3_DIP

[sjasonsmith]

I personally like compiler errors when incompatible options are used, so people know a feature isn’t going to work as expected.

[Bob-the-Kuhn]

Marlin has gone both ways on the decision of compiler error vs. change code. Scott will let us know which way he wants it. In this instance I don't have a strong opinion either way.

FYI - the functionality of SD_CHECK_AND_RETRY is already in the code enabled by USE_USB_COMPOSITE.

[thinkyhead]

True, we have a few different approaches on the menu…

• A sanity check most explicitly tells users all about the condition and makes them change it. Best approach when users need to know to be careful or they might blow something up.
• A conditional that silently (or with #warning) turns off the incompatible setting. OK as a middle ground when users ought to know for safety or for setup purposes.
• A conditional that silently disables the setting. Fine when an option is basically N/A and can be ignored.

In this case the option is sort-of N/A so silently disabling SD_CHECK_AND_RETRY makes sense. (In that case, the option means "bring in Marlin's own check-and-retry code.")

One other consideration is whether the chosen approach helps in "the Marlin encyclopedia project," where we treat Marlin as a "reference guide" to document the relationship between all these chips, their pins, common peripherals, libraries, and custom software routines all in one codebase. Considering that aspect it's better to have these kinds of checks in the 'Conditional' files — either the global one or just the affected HALs'.