ARMmbed / mbed-os-example-blinky

Blinky example for Mbed OS 6.0
Apache License 2.0
42 stars 157 forks source link

ACI: prevent compilation error for targets without any LED #265

Closed jeromecoutant closed 2 years ago

jeromecoutant commented 3 years ago

During mbed-os ACI, mbed-os-example-blinky is the basic application compiled for patch validation.

Some targets doesn't have any LED. Goal is to avoid any specific action for these target validation, this PR will make build OK, and indicate a warning (which can be tracked).

@MarceloSalazar

NB: this respects https://os.mbed.com/docs/mbed-os/v6.12/apis/standard-pin-names.html

MarceloSalazar commented 3 years ago

Thanks @jeromecoutant for the PR. The proposal lets users compile the app without having an LED, but also adds lots of noise in the main app. Perhaps we should just prevent the app from compiling if LED isn't defined as there isn't much value, or display a warning and ignore the run time behaviour?

#ifndef LED1
    #error / #warning "LED1 is not available."
#endif

@ARMmbed/mbed-os-maintainers thoughts?

jeromecoutant commented 3 years ago
#error / #warning "LED1 is not available."

No error, as I don't want CI to be failed...

jeromecoutant commented 2 years ago

Ping (needed for https://github.com/ARMmbed/mbed-os/pull/14831)

Thx

0xc0170 commented 2 years ago

What is ACI ? I dont think many users understand the comment in the code related to ACI.

I am not fond of

#ifndef LED1 // ACI usage
#warning "No LED for this target"
int main()
{
}
#else

in simple example like this. Do we have any other alternative? We can wrapp around only LED1 and just have led to be boolean that changes its value ? Any other suggestions?

jeromecoutant commented 2 years ago

Other alternative is to remove this blinky application build from mbed-os CI ? or check if LED1 exists before mbed-os CI ?

0xc0170 commented 2 years ago

CI has own configuration where you can add/exclude boards, I can ask.

What I had on my mind was:

#ifdef LED1
DigitalOut led(LED1);
#else
bool led;
#endif

Using bool should be valid as we just negate its value in the loop.

jeromecoutant commented 2 years ago

Yes, it works also! Done

0xc0170 commented 2 years ago

@ARMmbed/mbed-os-core Please review and integrate if all good. PR to mbed-os https://github.com/ARMmbed/mbed-os/pull/14831 depends on this