NFC APIs Design Specification #7426
Conversation
features/nfc/doc/nfc_design.md
Outdated
| ``` | ||
| These methods called when a remote initiator (the local controller is acting as a target) or a remote target (the local controller is acting as an initiator) is detected. | ||
|
|
||
| Shared pointers are used so that the user does not have to maintain the lifetime of these objects. The `NFCController` instance will release its reference when the endpoint is lost (see below). |
There was a problem hiding this comment.
If these objects are copyable then it might be better to pass them by value or const/reference and let the user code handle the copy if it is required.
There was a problem hiding this comment.
Thanks, I need to check if this is achievable but if so it makes sense.
features/nfc/doc/nfc_design.md
Outdated
|
|
||
| ```cpp | ||
| bool is_ndef_supported(); | ||
| size_t nfc_tag_type(); |
There was a problem hiding this comment.
Good point will switch to an enum
features/nfc/doc/nfc_design.md
Outdated
| bool is_ndef_supported(); | ||
| size_t nfc_tag_type(); | ||
|
|
||
| void set_ndef_message(const NDEFMessage& message); |
There was a problem hiding this comment.
There is mismatch with the diagram for set_ndef_message, clear_ndef_message, get_ndef_message.
Does it returns nfc_err_t ?
features/nfc/doc/nfc_design.md
Outdated
| virtual void on_deselected(); | ||
| ``` | ||
|
|
||
| Some phones/readers 'park' a target and re-select it later - these events let the user know if the what state the local target is being put in. |
features/nfc/doc/nfc_design.md
Outdated
| Some phones/readers 'park' a target and re-select it later - these events let the user know if the what state the local target is being put in. | ||
|
|
||
| ```cpp | ||
| virtual void on_before_ndef_read(); |
There was a problem hiding this comment.
Maybe the naming on_ndef_read_request and on_ndef_written would be more appropriate. I also wonder if the word message shouldn't be added to be more consistent with other delegates (see NFCTargetDelegate) What do you think ?
features/nfc/doc/nfc_design.md
Outdated
| A NDEF Message is made of multiple NDEF Records which is reflected by the API: | ||
|
|
||
| ```cpp | ||
| bool parse(const uint8_t* buffer, size_t sz) |
There was a problem hiding this comment.
What is the purpose of the parse function ? Does it parse or does it validate then copy an NDEFMessage from a byte stream ?
features/nfc/doc/nfc_design.md
Outdated
| All arrays are passed by reference (no copy made). | ||
|
|
||
| ```cpp | ||
| static bool parse(const uint8_t* buffer, size_t max_sz) |
features/nfc/doc/nfc_design.md
Outdated
| static bool parse(const uint8_t* buffer, size_t max_sz) | ||
| ssize_t build(const uint8_t* buffer, size_t max_sz) | ||
|
|
||
| uint8_t tnf() |
features/nfc/doc/nfc_design.md
Outdated
|
|
||
| The following events must be called to signal completion of long operations: | ||
| ```cpp | ||
| void on_backend_has_read_bytes(bool success, const uint8_t* bytes) |
There was a problem hiding this comment.
I think we can do better naming wise. I'm not sure that the on_backend prefix is required there.
Also I think it can be valuable to get a more descriptive error code.
Note: Shouldn't we get the size of the bytes read in the callback ?
features/nfc/doc/nfc_design.md
Outdated
| # Dependencies | ||
| * Event Queue | ||
|
|
||
| There are currently at least four event queues (Plaftorm, BLE, USB, IP) in mbed OS and NFC will also require an event queing mechanism. We should aim at reusing one of these existing queues with the long term goal of unifying these code bases. |
There was a problem hiding this comment.
This can be solved by not instantiating the event queue inside the NFC stack but rather let the application provide it.
Dependency injection delegate the layout of the program to the application writer. Ble already does that for this reason.
|
One think I forgot to mention is naming consistency towards event handler classes. Should it be It would be nice to pick one and stick with it at the OS scale. So it become a recognizable pattern. |
* Decouple Message parsing from Record parsing * Decouple record parsing from Common objects * Provide an easy to use parser that parse the most common objects.
NFC: Amend ndef parsing design.
|
@donatieng Making sure, is the idea that once this is done with review, that it will be merged into Mbed OS' master branch, closed, with code changes coming, or should this go into its own feature branch? |
|
@cmonr This one should land on master and will be followed by a subsequent PR for the implementation (targeting master as well). Aiming at having this one finalized by end of week. |
|
@pan- Please confirm if you are happy with this now ? |
|
@donatieng Please take a look and address @AnotherButler's comments. |
|
@AnotherButler thanks for the review, comments addressed. |
|
/morph build |
Build : FAILUREBuild number : 2979 |
|
Known failure, will be restarted once Connectivity drivers are in |
|
It's in! |
Build : SUCCESSBuild number : 2984 Triggering tests/morph test |
Test : SUCCESSBuild number : 2741 |
Exporter Build : SUCCESSBuild number : 2592 |
Description
This pull request includes the design specification for adding NFC APIs in mbed OS, with a support for card emulation and NFC EEPROMs and NDEF encoding/decoding.
The design document for review can be found here: Design Document
It follows guidelines defined in this PR: #7561.
Pull request type