Forked from updates by the-real-orca, added class for NTAG support #35
Conversation
…ing your android app even when it's not running.
…k in source for more info. These two files will override the new and delete operators to use C-functions. This will drastically decrease code size. Signed-off-by: Christoph Tack <[email protected]>
Signed-off-by: Christoph Tack <[email protected]>
Signed-off-by: Christoph Tack <[email protected]>
Signed-off-by: Christoph Tack <[email protected]>
Signed-off-by: Christoph Tack <[email protected]>
Conflicts: Ndef.h NdefRecord.cpp
Conflicts: MifareUltralight.cpp Ndef.h
Conflicts: NdefMessage.h
No payload flag lets you still examine/access header bytes. Create example showing this, but no unit tests.
Not tested but demonstrated in NdefWithoutPayload example.
I think the tests were broken (not up to date with refactored functions) when I forked the library, so I don't know whether these tests previously passed.
Work in progress.
…All tests pass, with coverage of header sizing only.
…dware attached). All tests pass.
… tests pass but some cleanup needed.
…here bit width of sizes was 8 when 16 needed.
…t happened on master. # Conflicts: # NdefMessage.cpp # NdefMessage.h # NfcTag.cpp # ntag.cpp # ntag.h # tests/NdefMessageTest/NdefMessageTest.ino
…NDEF. More type conversion to uint8_t and uint16_t.
| return false; | ||
| } | ||
| } | ||
|
|
||
| void NdefMessage::addMimeMediaRecord(String mimeType, String payload) | ||
| void NdefMessage::addMimeMediaRecord(const char *mimeType, const char* payload) |
There was a problem hiding this comment.
const char * might be technically better, but String is preferred for Arduino libraries because it's easier for beginners
There was a problem hiding this comment.
Agreed. I may have run into out of memory problems in my own applications with this library, especially on ATmega328P, and avoiding Strings was a way to mitigate this.
| { | ||
| addTextRecord(text, "en"); | ||
| addTextRecord(text, ""); |
There was a problem hiding this comment.
Can't remove the language code 'en', it's required for text records. Ideally it would default to en and allow users to override.
There was a problem hiding this comment.
I don't remember why I did this. Possibly because it was confusing me and I wanted a more manual way of writing the data, which looks like it should have been to call the 2-argument version of this function with the second argument empty.
Restoring the default encoding here will probably require changes to a handful of calls in unit tests, i.e. from addTextRecord(text) to addTextRecord(text, "")
|
@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. |
|
@don |
|
@eecharlie I'll see if I can extract some of the stuff from your branch. If i run into trouble I'll ask questions. |
|
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 |
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