search

ruuvi / ruuvi.drivers.c

Ruuvi embedded drivers used across various platforms. Used by ruuvi.firmware.c. In Beta, tests which pass will not be broken without incrementing major version.
BSD 3-Clause "New" or "Revised" License
6 stars 14 forks source link

Incorrect use of P_LANG_CODE in NFC #7

Open mdxs opened 5 years ago

mdxs commented 5 years ago

It appears that the P_LANG_CODE fields of the NDEF text records used in NFC are custom identifiers: "fw", "ad", and "id"; instead of the IANA language code describing the language used (see https://www.nordicsemi.com/DocLib/Content/SDK_Doc/nRF5_SDK/v15-2-0/group__nfc__text__rec).

https://github.com/ruuvi/ruuvi.drivers.c/blob/e4c90c6ec09b794c65714aab982c86f81a4a4cef/nrf5_sdk15_platform/communication/ruuvi_platform_communication_nfc.c#L154

https://github.com/ruuvi/ruuvi.drivers.c/blob/e4c90c6ec09b794c65714aab982c86f81a4a4cef/nrf5_sdk15_platform/communication/ruuvi_platform_communication_nfc.c#L162

https://github.com/ruuvi/ruuvi.drivers.c/blob/e4c90c6ec09b794c65714aab982c86f81a4a4cef/nrf5_sdk15_platform/communication/ruuvi_platform_communication_nfc.c#L170

This should therefore probably be changed to use "en" for all three NDEF text records?

ojousima commented 5 years ago

This is actually intentional, as there is no standard way to let user know that they're reading FW version, MAC address and ID if the device.

mdxs commented 5 years ago

there is no standard way to let user know that they're reading FW version, MAC address and ID of the device

It appears that in the task_nfc_init() method inside ruuvi/ruuvi.firmware.c/tasks/task_nfc.c you prefix the field values with "FW: ", "MAC: ", and "ID: " inside the P_DATA of the NDEF text records.

So while there might not be a standard way, it seems that currently you opt to try and do it twice. The P_DATA based prefixes (of "FW: ", "MAC: ", and "ID: ") are fine, the P_LANG_CODE usage (of "fw", "ad", and "id") seems to be going against the standard defined for P_LANG_CODE.

In fact, the text of the "ID: " prefixed field is according to the P_LANG_CODE "id" showing that it is provided in the "Bahasa Indonesia" language.

Just wanted to point it out, but recognize that this might be a deliberate choice at this point in time. So feel free to feel informed and close this issue.

ojousima commented 5 years ago

Now hen I think about it, the NFC driver should not enforce those fields anyway. It ties the drivers to a specific application use case in a way that having separate drivers tries to avoid. I won't do any fix in near-term, but I'll keep this issue open as a reminder to refactor it somehow.