Remove float symbols from apps that use the NFCController class

[hugueskamba]

Summary of changes

Use the Ticker API that does not use floating point types and literal.
Also pass timeout in seconds instead of incorrectly passing milliseconds.

Impact of changes

Migration actions required

Documentation

---

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

@kjbracey-arm @evedon

---

[kjbracey]

Why is the forwarding being dropped? It reduced code size (by avoiding a relatively-expensive Callback->Callback copy). Does it have any bearing on the float elimination?

[hugueskamba]

No it does not. I was checking if float elimination needed that. I will put it back for code size reduction.

[kjbracey]

This breaks the API for anything >2000s. Is it the limiting to 32-bit that is making the difference for your test?

And that test is a non-constant use, I gather? Someone passing a non-constant integer to this?

If trying to avoid float use, you should fix the calling code to not use floats, not break this API.

[hugueskamba]

Changing the calling code as follows without making any changes to Ticker.h does not remove float symbols.

void NFCController::scheduler_process(bool hw_interrupt)
{
    _timeout.detach(); // Cancel timeout - if it triggers, it's ok as we'll have an "early" iteration which will likely be a no-op

    // Process stack events
    uint32_t timeout_ms = nfc_scheduler_iteration(_scheduler, hw_interrupt ? EVENT_HW_INTERRUPT : EVENT_NONE);

    _timeout.attach(
        callback(this, &NFCController::on_timeout),
        timeout_ms/1000 // convert milliseconds to seconds <=====
    );
}

[kjbracey]

Well, no, because you're still using the attach(float).

That should be

_timeout.attach_us(cb(blah), timeout_ms * (us_timestamp_t) 1000);

(Cast needed to ensure it's a 64-bit multiply, not a 32-bit one).

[hugueskamba]

Done.

Pull request has been modified.

[ciarmcom]

@hugueskamba, thank you for your changes.
@kjbracey-arm @evedon @ARMmbed/mbed-os-pan @ARMmbed/mbed-os-maintainers please review.

[evedon]

Simple change at the end :-)

You might want to repeat the title in the Impact of change section.

[kjbracey]

The description is a bit backwards - the main change is now the 1000 times scheduling speed-up, not the removal of float use, even if it's not the reason you approached it in the first place.

As such, it should be reviewed by @donatieng or @pan- or someone in NFC area. I can only assume there was a lack of test coverage here - nothing actually needing to trigger any retries?

[0xc0170]

CI started meanwhile

[mbed-ci]

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

[0xc0170]

As such, it should be reviewed by @donatieng or @pan- or someone in NFC area. I can only assume there was a lack of test coverage here - nothing actually needing to trigger any retries?

Set this one for review stage

[pan-]

LGTM