Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow NDEF records with length over 254 bytes #2407

Merged
merged 4 commits into from
Aug 30, 2023

Conversation

fanoush
Copy link
Contributor

@fanoush fanoush commented Aug 25, 2023

  • use 3 byte length in NDEF TLV block when over 254 bytes
  • store 16bit url payload length
  • -add maximum lenght checking (type 2 tag data is up to 992 bytes)

this is first attempt to fix #2406 , works for me

- use 3 byte length in NDEF TLV block when over 254 bytes
- store 16bit url payload length
-add maximum lenght checking (type 2 tag data is up to 992 bytes)
@fanoush
Copy link
Contributor Author

fanoush commented Aug 25, 2023

not sure about NDEF and type 2 tag terminology in the comments, feel free to fix/suggest
also the NRF.nfcUrl has some duplicated code with NRF.nfcRaw, it could be done like others - prepare just the payload and pass that to jswrap_nfc_raw

- move common code from jswrap_nfc_raw into  nfc_raw_data_start and reuse it in all NRF.nfc methods
- also avoid creating one extra flat string copy of the payload
@fanoush
Copy link
Contributor Author

fanoush commented Aug 28, 2023

After splitting jswrap_nfc_raw into two parts the second part can be more easily reused from all other methods without creating extra copy of the payload.

Maybe I should move the nfc_raw_data_start method elsewhere now? at least above the first use so the extra definition is not needed there?

EDIT: and BTW the cleanup saved some code so even with new functionality it is smaller than before (by 136 bytes)
https://github.com/espruino/Espruino/actions/runs/6005580136/job/16288554523?pr=2407#step:3:1137
https://github.com/espruino/Espruino/actions/runs/5903401846/job/16013282040#step:3:1137

@gfwilliams
Copy link
Member

This looks great, thanks! Really good news on the space saving too!

Maybe I should move the nfc_raw_data_start method elsewhere now? at least above the first use

That sounds like a good plan - maybe just to where you put the forward decl for now?

@fanoush
Copy link
Contributor Author

fanoush commented Aug 29, 2023

Moved. Also double checked the length check. This works NRF.nfcURL("http://espruino.com?param="+'x'.repeat(959)+'z') and when reading back on phone with NXP TagInfo it fills data area completely and leaves just 16 bytes at the end of the 1024 bytes just as described here https://infocenter.nordicsemi.com/topic/com.nordic.infocenter.sdk5.v14.0.0/nfc_type2tag_format_dox.html?cp=9_5_7_3_27_1_1 One more character gives Url too long.

@gfwilliams
Copy link
Member

That's excellent, thanks! Merging now :)

@gfwilliams gfwilliams merged commit 032fbc2 into espruino:master Aug 30, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NRF.nfcRaw (and nfcUrl) limited to payload/message length 255
2 participants