don / NDEF

NDEF Library for Arduino. Read and Write NDEF Messages to NFC tags with Arduino.
Other
284 stars 142 forks source link

Forked from updates by the-real-orca, added class for NTAG support #35

Open eecharlie opened 7 years ago

eecharlie commented 7 years ago

This is my first time submitting a pull request on Github!

I brought in an ntag class originally by LieBtrau that I thought did well as included in this library. I made updates to it to facilitate creating and writing NDEF messages to NXP 'ntag' nfc chips easily, with a direct I2C interface. I also fixed a number of bugs in the existing NDEF message & record classes that appear as soon as things get longer than 8-bit indexing can handle.

There is an added dependence on a Bounce2 button debounce library that I don't love, and some commented out I2C diagnostic code, but otherwise I think this is in good shape and well tested.

Charlie

don commented 6 years ago

@eecharlie Thanks for the pull request. Since I've merged #21 there's conflicts with this.

Unfortunately this pull request is huge and changes a ton of things. Generally multiple smaller pull requests are better. I'm going to leave the pull request open since I'd like to manually add NTAG support from your branch, but I don't want stuff where you change the function signatures.

The static buffer stuff looks interesting. Debounce support is good. I need to look into the encoding fixes for indexes > 8-bit. I'm interested if you have cases that demonstrate the bugs.

eecharlie commented 6 years ago

@don It's been a while since I've looked at this code, but I'm happy to go back and see if I can address your questions. What should the process look like from here - is the ball still in your court, other than me answering your specific questions above? -Charlie

don commented 6 years ago

@eecharlie I'll see if I can extract some of the stuff from your branch. If i run into trouble I'll ask questions.

eecharlie commented 6 years ago

Re: 8-bit vs 16-bit indexing, taking a quick look at my commits in which I did this, I may have written a unit test around this, see test(big_record_handling) in NdefMessageTest.ino