devanlai / dapboot

DFU Bootloader for STM32 chips
Other
337 stars 111 forks source link

Overriding target_get_force_bootloader #46

Open obra opened 2 years ago

obra commented 2 years ago

For my application, we want to inhibit the ability for software to force the bootloader to reboot into programming mode without physical user interaction. To that end, I think I need to override target_get_force_bootloader. By default, that function isn't marked as weak. Is the right change as follows? If so, I'll submit a PR. (Come to think of it, I'll want to do this with target_clock_setup, too)

modified: src/target.h
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@@ -26,7 +26,7 @@
 extern void target_clock_setup(void);
 extern void target_gpio_setup(void);
 extern const usbd_driver* target_usb_init(void);
-extern bool target_get_force_bootloader(void);
+extern bool target_get_force_bootloader(void)  __attribute__((weak));
 extern void target_get_serial_number(char* dest, size_t max_chars);
 extern size_t target_get_max_firmware_size(void);
 extern void target_log(const char* str);
devanlai commented 2 years ago

I've only been marking functions as weak as a convenience for some of hooks that have default / no-op implementations in dummy.c.

In general, to customize the behavior for a target, I would either add more #ifdef controlled code to target_stm32f103.c or create a new target directory if the changes are not broadly applicable or too extensive to put in the common implementation.

obra commented 2 years ago

Just in terms of extensibility without butchering the core code, I've been finding the weak functions really nice. It's meant that I can extend the code for a weird use case without junking up the core implementation and still keep the cut-and-paste duplication to a minimum.

devanlai commented 2 years ago

Weak functions work okay if there's only one level of overriding necessary, but IIRC, it's not defined what happens if you provide multiple weak definitions of a symbol to the linker. I agree that it is nice to be able to extend things without modification, but there are only so many hacks we can get away with before we end up building a poor version of C++.

obra commented 2 years ago

Understood.