fix: support using ReadURI with SMS and GEO type

[maihde]

If you write a GEO or SMS URI with the WriteURI function, for example:

st25dv.writeURI("", "sms:+17035946155", "")

It will fail when using ReadURI because NDEF_ParseURI has special logic for looking at the URI string and changing the NDEF_Type. For symmetry in the API and to make ReadURI more versatile I have added support to read the SMS and GEO types with ReadURI.

[cparata]

Hi @maihde ,
first of all, thanks for the contribution! I just have few comments. Why did you not use the APIs available in the NDEF library? There are dedicated APIs for SMS and GEO with helper functions in the files lib_NDEF_SMS.cpp/h and lib_NDEF_Geo.cpp/h. I would reuse those APIs to extend the features of the library. What do you think about it?
Best Regards,
Carlo

[maihde]

@cparata yes, I agree that the NDEF library APIs are available. This PR is primarily about ensuring symmetry with the API, it seemed to be unintentional that writeURI can write an abridged URI and then an immediate call to readURI would fail. I also think it's more intuitive to use readURI for these simple cases and prevents users from having to write code to handle various cases where they may not know what type of URI is on the tag. Finally, it also seemed pretty low-risk since no new APIs were introduced and it seemed unlikely that someone would have software relying on readURI to failing in this situation.

[cparata]

Hello @maihde ,
I would prefer to add dedicated high level APIs to support other data formats besides writeURI/readURI . In my opinion, the correct path should be using the low level APIs that are already available in the NDEF library in the files lib_NDEF_SMS.cpp/h and lib_NDEF_Geo.cpp/h to implement new high level APIs like writeSMS/readSMS and writeGeo/readGeo. Unfortunately, I do not have band to work on it, but it would be great if the community gives his contribution to improve the library.
Best Regards,
Carlo

[maihde]

Carlo, I don't mind adding and contributing those higher level APIs as you suggested. One question, should `writeURI` fail if provided an SMS or GEO URI? Currently the writeURI succeeds but the readURI fails, which IMHO is counter-intuitive.

…

On Tue, Jul 25, 2023 at 4:00 AM Carlo Parata ***@***.***> wrote: Hello @maihde <https://github.com/maihde> , I would prefer to add dedicated high level APIs to support other data formats besides writeURI/readURI . In my opinion, the correct path should be using the low level APIs that are already available in the NDEF library in the files lib_NDEF_SMS.cpp/h and lib_NDEF_Geo.cpp/h to implement new high level APIs like writeSMS/readSMS and writeGeo/readGeo. Unfortunately, I do not have band to work on it, but it would be great if the community gives his contribution to improve the library. Best Regards, Carlo — Reply to this email directly, view it on GitHub <#30 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABYEIXCQZ5FFDTQQNNFJFTXR54KLANCNFSM6AAAAAA2T6CSLU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[cparata]

Hello @maihde ,
in my opinion, you are trying to use writeURI and readURI in a way that they were not designed for. For SMS, Geo and Email there are dedicated low level APIs with specific structs. So, I prefer to see high level APIs based on the helper low level functions for the other specific cases.
Best Regards,
Carlo

[maihde]

Thanks @cparata! I don't wish to be a bother, so I'll assume the desire to have no change in behavior for writeURI and readURI unless I hear otherwise.

However, I wanted to provide a few more details since I may not be articulating the question well.

The existing behavior allows code like this.

// Write an abridged URI compatible with Apple URL background tag schemes
//https://developer.apple.com/documentation/corenfc/adding_support_for_background_tag_reading

// Abridged Prefix
st25dv.writeURI("mailto:", "[email protected]", "") // NDEF_OK
st25dv.readURI(&uri_read) // NDEF_OK

st25dv.writeURI("tel:", "+14085551212", "") // NDEF_OK
st25dv.readURI(&uri_read)  // NDEF_OK

// 'none' prefix
st25dv.writeURI("", "sms:+14085551212", "") // NDEF_OK
st25dv.readURI(&uri_read) // NDEF_ERROR

st25dv.writeURI("", "facetime://[email protected]", "") // NDEF_OK
st25dv.readURI(&uri_read) // NDEF_OK

st25dv.writeURI("", "X-HM://[email protected]", "") // NDEF_OK
st25dv.readURI(&uri_read) // NDEF_OK

// Not supported by iOS
st25dv.writeURI("", "geo:12345.123,12345.123", "") // NDEF_OK
st25dv.readURI(&uri_read) // NDEF_ERROR

How would you like writeURI to work?

• Should writeURI succeed in all cases? (current behavior in baseline version)
• Should writeURI return NDEF_ERROR for sms and geo?
• Should writeURI return NDEF_ERROR for all arguments where the first argument is blank? (this would prevent writing custom URIs)

[cparata]

Hello @maihde ,
I think that using the writeURI without specifying any protocol is out of the scope of the writeURI and probably we could return an error when we use the API in this way.
Best Regards,
Carlo

[maihde]

@cparata I have pushed an update that adds the following API to the ST25DV class:

• writeUnabridgedURI/readUnabridgedURI - Support for NDEF protocol code 0x00 which means no URI prepending is performed. This allows users of the ST25DV to write URIs that are supported by Android/iOS devices but not part of the NDEF standard
• write[SMS, GEO, EMail]/read[SMS, GEO, Email] - Support to read/write common URI types without having to use the NDEF class or NDEF structures
• readNDEFType - When dealing with a tag where you don't know the type, this provides a simple high-level call for users that doesn't require them to use the NDEF object.

This code is based on fixes from #31 because the refactoring of NDEF_IdentifyNDEF makes the implementation of the above more concise. If you want to be able to pull these changes without also pulling #31 I can redo this PR.

[cparata]

Hello @maihde ,
thanks a lot for this contribution. We just need to fix AStyle and Codespell issues to pass CI check. Regarding Codespell, you can see the report here and the issue should be easily fixed. Regarding AStyle issues, you just need to run a script that fixes all code style issues as explained in this guide.
After these fixes, I can integrate the PR in the main branch.
Thanks again and Best Regards,
Carlo

[cparata]

Hi @maihde ,
thanks a lot for your contribution. PR integrated.
Best Regards,
Carlo