JAndrassy / ArduinoOTA

Arduino library to upload sketch over network to Arduino board with WiFi or Ethernet libraries
GNU Lesser General Public License v2.1
435 stars 89 forks source link

Define `NO_OTA_NETWORK` to disable all network-related functionality #191

Closed vshymanskyy closed 1 year ago

vshymanskyy commented 1 year ago

I've developed a custom firmware delivery mechanism (i.e. using a cellular or packet radio connection on an ESP32), so I'd like to disable the built-in implementation.

JAndrassy commented 1 year ago

the change in cpp does nothing

vshymanskyy commented 1 year ago

It should disable the compilation unit, optimizing the build speed and not relying on the link-time code elimination. Should I revert this change?

JAndrassy commented 1 year ago

It should disable the compilation unit, optimizing the build speed and not relying on the link-time code elimination. Should I revert this change?

the cpp will always be compiled and it will not see the define from ino or other cpp. It should be enough to change in ArduinoOTA.h

#ifdef NO_OTA_NETWORK 
// don't create ArduinoOTA instance

#elif defined(ethernet_h_) || defined(ethernet_h) // Ethernet library
vshymanskyy commented 1 year ago

Ah, I see what you mean, but I'm actually using PlatformIO so it will see the define (build_flags get applied to all the libraries as well)

On Mon, 13 Mar 2023, 12:42 Juraj Andrássy, @.***> wrote:

It should disable the compilation unit, optimizing the build speed and not relying on the link-time code elimination. Should I revert this change?

the cpp will always be compiled and it will not see the define from ino or other cpp. It should be enough to change in ArduinoOTA.h

ifdef NO_OTA_NETWORK

// don't create ArduinoOTA instance

elif defined(etherneth) || defined(ethernet_h) // Ethernet library

— Reply to this email directly, view it on GitHub https://github.com/JAndrassy/ArduinoOTA/pull/191#issuecomment-1465900121, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALP3FHAOOKPNWM43X6BTBDW332Y7ANCNFSM6AAAAAAVYIGYHE . You are receiving this because you authored the thread.Message ID: @.***>

JAndrassy commented 1 year ago

if you write a custom firmware in PlatformIO you could just include InternalStorage.h

vshymanskyy commented 1 year ago

As I'm building for multiple platforms, essentially I need to copy this section of code: https://github.com/JAndrassy/ArduinoOTA/blob/ee82be99cf2777636ddb709306172dbf6be44e97/src/ArduinoOTA.h#L23-L35 This could be moved to a separate header, in this case I could include it directly.

JAndrassy commented 1 year ago

you can copy that section to your firmware and remove the lines for platforms your firmware doesn't support. then it is only a few lines

btw: I am curios now. First I thought you work on Edgent improvements, but that would require Arduino IDE compatibility. Then I thought you make some IoT firmware for a big customer, but that would be one platform. :-)

vshymanskyy commented 1 year ago

Yeagh, copying those lines is what we do now, but at the same time it's something I'd like to avoid. I.e. ArduinoOTA could get updated at any point with new platform support, at which point it would require changes in our libraries as well.

You're correct, ArduinoOTA will be compiled in Arduino IDE in many cases, I just thought it's a good opportunity to improve the PlatformIO developer experience as well.

It's related to the Edgent development (and to customers of all sizes), however we're not disclosing the details yet. ;)

JAndrassy commented 1 year ago

ok, then, as I write in one of the previous comments, I would prefer to have just the #ifdef NO_OTA_NETWORK before the #elifs for the networking libraries. this basically just suppresses the warning in the final #else and/or ignores any networking library included before ArduinoOTA.h.

vshymanskyy commented 1 year ago

Works for me. Just reverted the other changes