Fix MBED_ERROR call in init_os_timer

[AGlass0fMilk]

Summary of changes

If a target is configured without support for LPTICKER or USTICKER (pretty much never happens), a call to the MBED_ERROR macro is made that uses an old version of the error macro API.

This PR updates the call to be consistent with the new error macro API.

Impact of changes

Builds without LPTICKER nor USTICKER enabled will no longer fail due to incorrect MBED_ERROR macro usage in init_os_timer.

Migration actions required

None

Documentation

None

---

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

---

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

---

Reviewers

---

[ciarmcom]

@AGlass0fMilk, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

[0xc0170]

@AGlass0fMilk Please review astyle failures

[0xc0170]

@AGlass0fMilk shall we close this one?

[evedon]

Thanks for updating the MBED_ERROR macro but it is safer to report the error even if MBED_TRAP_ERRORS_ENABLED is not set. This path is not valid. If there is an RTOS, then the OS timer needs to work. In bare metal, it's all fine until a timing function is used. A run-time error in this function is more useful than a null pointer exception later on.

[AGlass0fMilk]

I have removed the MBED_TRAP_ERRORS_ENABLED if preprocessor so this error occurs if timing functions are used in baremetal regardless of error-masking configuration.

[LDong-Arm]

If there is an RTOS, then the OS timer needs to work.

In a case (e.g. RTOS) where a functionality is a strict requirement, a compile time error (e.g. #error) makes sense. Though we don't support RTOS (i.e. the full profile) on targets without timers to begin with I guess.

[kjbracey]

There are 4 cases here:

• RTOS in non-tickless mode. Uses SysTick peripheral, does not use OsTimer
• RTOS in tickless mode. Uses OsTimer to generate timebase from usticker or lpticker.
• Non-RTOS with timing calls. Uses OsTimer, inited on first use
• Non-RTOS with no timing (eg bootloader). OsTimer not needed; it and the ticker system should be linker excluded.

I agree a compile-time error makes sense, if there isn't one already, but this piece of code shouldn't really know the intricacies of who needs the OS timer. Not the place to put a bunch of "if RTOS and not tickless" defines. It's the lower layer.

You could arrange for OsTimer and init_os_timer to only be declared and defined if the timers were available, leading to compile errors on attempt to use, but those would not be terribly clear errors, and you'd have to add a whole bunch of ifdefs to existing code, meaning the problem cascades up somewhat.

With the updated APIs that are split between timing and non-timing (eg Semaphore::acquire/try_acquire versus Semaphore::try_acquire_for) you could have the same conditionality - try_acquire_for not available if no RTOS and no OsTimer.

But there are other calls that haven't had that split like EventQueue::dispatch() where dispatch(-1) or dispatch(0) is legal without the timer, but dispatch(100) isn't. (Actually, due to implementation details, it currently isn't, but it could be). So they'd end up just being run-time checks. Those APIs should also be split - the split is what allows clean linker exclusion of timing code in bootloaders and other cut-down stuff.

Anyway, a happy medium would be that init_os_timer continues to exist and give a run-time error, to catch non-RTOS users going down a "needs timing" path. But there could be a define has matching the runtime condition like OSTIMER_AVAILABLE and the RTOS code could check that: #if MBED_TICKLESS && !OSTIMER_AVAILABLE / #error.

That keeps the layering clean - the RTOS doesn't need to know how OSTimer is getting its time, and all the lpticker/usticker stuff, and OSTimer dosn't need to care about RTOSes.

I think that's a separate patch though - this PR is fine as-is.

Pull request has been modified.

[AGlass0fMilk]

@0xc0170 I have addressed the astyle failure and @evedon's comments.

This should be good to go if it will be merged at all.

[0xc0170]

CI started

[mbed-ci]

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage	✔️
jenkins-ci/mbed-os-ci_cmake-example-test	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest	✔️