Add source files #1

ghost · 2017-10-02T11:23:30Z

Add source files to M24SR64-Y library.
Examples available for Discovery L475VG IOT board.

This PR removed an issue in this library.

fpistm

First quick review.

General comment:
Please review indentation, sometimes tab, sometimes 2 spaces or more. Preferred 2 spaces instead of tab (same as core).
Review also indentation alignments.

Thanks

fpistm · 2017-10-02T11:52:09Z

README.md

+The WriteURIMail sketch writes a Mail tag on the device. It records a mail with the recipient, the subject and the body of the message.
+the WriteText sketch writes a Text tag on the device. It records a simple text message.
+
+We can see on the monitor window the message "Sytstem init done!" when the NFC module is started and ready.


Reword (please avoid use of "we"):
When the NFC module is started and ready, the message "Sytstem init done!" is displayed on the monitor window.

fpistm · 2017-10-02T11:52:51Z

README.md

+You can't write tag with a total size (protocol + payload) greater than 246 bytes.
+This is the limitation one Iblock command on M24SR device. This limitation is also
+in the variant.h file, the I2C buffer have a limited size.
+This library was tested on DISCO_L475VG_IOT_board, it's may not work on other board.


It should work with each board including M24SR? or using the NFC shield?
So?

Of course but we tested only on DISCO L475 IOT. The sentence will be removed.

fpistm · 2017-10-02T11:59:07Z

examples/WriteAAR/WriteAAR.ino

+const uint8_t M24SR_ADDR=0xAC;
+const int     DEFAULT_GPO_PIN=PE4;
+const int     DEFAULT_RF_DISABLE_PIN=PE2;
+


All those parameters are dedicatedt toL4 disco IOT. Add a comment to mention that user have to update them according to the board tested and that those one are for L4 IOT

fpistm · 2017-10-02T11:59:39Z

examples/WriteMime/WriteMime.ino

+* Choose the type, the data and the length of the Mime by changing
+* the content of mime_type and data, the length is automatic.
+*
+* We can see on the monitor window the message "Sytstem init done!" when the


fpistm · 2017-10-02T11:59:53Z

examples/WriteAAR/WriteAAR.ino

+* Check the pinout and the I2C interface used for communication.
+* Choose the name of the application by changing the content of text_write.
+*
+* We can see on the monitor window the message "Sytstem init done!" when the


fpistm · 2017-10-02T12:40:36Z

src/NDefNfcTagM24SR.h

+  ******************************************************************************
+  */
+
+#ifndef X_NUCLEO_NFC01A1_M24SR_NDEFNFCTAGM24SR_H_


remove X_NUCLEO_NFC01A1_M24SR

fpistm · 2017-10-02T12:41:00Z

src/NDefNfcTagM24SR.h

+
+};
+
+#endif /* X_NUCLEO_NFC01A1_M24SR_NDEFNFCTAGM24SR_H_ */


fpistm · 2017-10-02T12:47:16Z

src/m24sr_def.h

+
+#endif /* __DRV_M24SR_H */
+
+/******************* (C) COPYRIGHT 2013 STMicroelectronics *****END OF FILE****/


Remove date to avoid misalignment

fpistm · 2017-10-02T12:48:08Z

examples/WriteAAR/WriteAAR.ino

+* You can also write an AAR tag with your smartphone.
+* On Android, donwload the NFC Tools.
+* Then start the app, check if NFC is activated on your smartphone.
+* Choose "ecrire" and then select the AAR tag.


And for english smartphone?

fpistm · 2017-10-02T12:52:10Z

README.md

+## Important note
+
+Before using this library, install the ArduinoSTL library.  
+Make sure to include the patch number with commit


Seems difficult to apply for non github user.

I agree but we have no choice.
@cparata push a PR but we are waiting the merge.

Please, use this PR to test: stm32duino/Arduino_Core_STM32#129
This allow to use STL without Arduino STL.
Build ok by removing all ArduinoSTL.h include in NFC sketches.

VVESTM

Hi,

I made a first test with nfc and I have bad results. See my comments on examples/WriteText/WriteText.ino, they apply to all examples.

To test, I have :
* This PR
* stm32duino core + stm32duino/Arduino_Core_STM32#120
* ArduinoSTL lib + mike-matera/ArduinoSTL#12

VVESTM · 2017-10-02T15:08:38Z

examples/WriteText/WriteText.ino

+* If you want to read tag in RF mode (with smartphone) unplug the USB on the board.
+* The board should be power-off, or RF mode will not be available.
+*
+* You can write a Text tag with your smartphone.


Description not clear. In this case, the sample will write a text on the chip, then, it will read it to check if it is OK. With a smartphone, we will be able to check that text has been updated.
The read / write functionality from the smartphone is not dependent of the Arduino sketche...

VVESTM · 2017-10-02T15:09:40Z

examples/WriteText/WriteText.ino

+
+void loop() {
+  const char *text_write = "hello world welcome on earth";     // Text to write in the tag
+  char text_read[255];                                         // Text read in the tag


It can be good to initialize this value. (in case we see something in case of error during reading)

VVESTM · 2017-10-02T15:11:28Z

examples/WriteText/WriteText.ino

+    //read the txt message and print it
+    nfcTag.readTxt(text_read);
+    SerialPort.println("Message content :");
+    SerialPort.println(text_read);


Need a strcmp to compare to the written message.

In my case, the read message is always empty... Perhaps I missed something in the way to test the NFC, but even in this case, we should get an error.

One other change, is to perform the test in the setup as it is done only one

@VVESTM

There is a problem with the library. My release was ready since the last week before the PR of the I2C on IT. So it broke the library communication.
I must find a patch...

Sounds strange that the I2C IT broke it.

Perhaps it is a mismatch between i2c working with IT now and i2c buffer size. There are still I2C_TXRX_BUFFER_SIZE defined to 32 for example...
(I have tried to set it to 255 but it is not working).

Then, as mentioned by @fpistm, it can be good to have something more flexible. (Avoid to use a big buffer if not needed)

I confirm the problem is in the I2C layer. It is not the buffer size the problem.

From the datasheet of the module we can read:

A restart during a command and response exchange is not supported by the M24SR64-Y.
After a command, the I2C host can execute a Polling sequence to determine when the
response is available.
Polling sequence: Loop on START (S) + DeviceSelect with RW=0 + read NACK/ACK + STOP(P)
The response is available as soon as the M24SR64-Y sends an ACK ( host read will a '0').

That means the I2C goes in READY state but with AF (Acknowledge failure) error when it is waiting a response from the module because the module returns NACK. As the error code is not checked then the Wire library returns OK to the NFC driver but the module it is not ready yet!

The quick solution is to check the errorCode parameter in i2c_master_read(). But I saw other problems in the twi.c file. I will open an issue to expose to you those problems. (And I know I validated the PR but too quickly I think).

My 2 cents:
I think it is ok to exit i2c_master_read as soon as I2C READY internal state is reached
To check whether device is ready before sending a new command, the below API shall be used:
i2c_status_e i2c_IsDeviceReady(i2c_t *obj, uint8_t devAddr, uint32_t trials)
It is up to the device driver to use this API when needed (as recommended by spec) before sending a new I2C command. I have seen EEPROM where is sometimes takes 5 or 6 checks before the device is actually ready ...

Signed-off-by: fpr <[email protected]>

ghost · 2017-10-12T14:53:30Z

PR updated:

fixes remarks from @fpistm and @VVESTM
works with STL PR. Library ArduinoSTL isn't required anymore.
I2C reworked to use this PR. This PR can be definitively abandoned and closed.

However, this PR needs to be validated with the fix of this issue before to be merged.

ghost · 2017-10-17T12:35:50Z

Updated to work with I2C IT mode.

ghost · 2017-11-07T08:41:37Z

Library updated with the last modification of the Wire library (PR).

Signed-off-by: fpr <[email protected]>

ghost · 2017-11-08T14:23:49Z

Updated to match with the latest version of this PR.

cparata

I would like to add in the examples also the settings for X-NUCLEO-NFC01A1 as we did for SPBTLE-RF library, so we have a porting that works fine with both Discovery IoT Node and X-NUCLEO-NFC01A1.

I decided to create a dedicated package for X-NUCLEO-NFC01A1 derived from M24SR64-Y library.

fpistm

ok for me. Wait merge of stm32duino/Arduino_Core_STM32#131 before merge this one

fpr added 2 commits October 2, 2017 10:00

Add library properties

024a799

Signed-off-by: fpr <[email protected]>

Add source files from x-nucleo-nfc01a1

6dce63d

Signed-off-by: fpr <[email protected]>

ghost requested review from cparata, LMESTM, fpistm and VVESTM October 2, 2017 11:24

ghost mentioned this pull request Oct 2, 2017

Remove I2C buffer length limitation required by M24SR64-Y library stm32duino/Arduino_Core_STM32#120

Closed

fpistm requested changes Oct 2, 2017

View reviewed changes

VVESTM reviewed Oct 2, 2017

View reviewed changes

Clean spaces

e89183b

Signed-off-by: fpr <[email protected]>

ghost requested review from fpistm and VVESTM October 12, 2017 14:53

fpr added 9 commits November 8, 2017 14:56

Add delay to avoid an error with I2C IT

96939af

Signed-off-by: fpr <[email protected]>

Add text tag

7970244

Signed-off-by: fpr <[email protected]>

Add URI tag

a2ac118

Signed-off-by: fpr <[email protected]>

Add AAR tag

bf3fb27

Signed-off-by: fpr <[email protected]>

Add URI mail tag

af73209

Signed-off-by: fpr <[email protected]>

Add SMS tag

2f446ae

Signed-off-by: fpr <[email protected]>

Add Mime tag

2a1e521

Signed-off-by: fpr <[email protected]>

Add syntax coloring map

e2363e5

Signed-off-by: fpr <[email protected]>

Update README.md

a724852

Signed-off-by: fpr <[email protected]>

cparata previously requested changes Nov 8, 2017

View reviewed changes

fpistm reviewed Nov 14, 2017

View reviewed changes

fpistm approved these changes Nov 16, 2017

View reviewed changes

fpistm merged commit 8a03355 into stm32duino:master Nov 16, 2017


		#endif /* __DRV_M24SR_H */

		/***************** (C) COPYRIGHT 2013 STMicroelectronics *END OF FILE**/

Add source files #1

Add source files #1

Uh oh!

Conversation

ghost commented Oct 2, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fpistm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VVESTM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VVESTM Oct 2, 2017 • edited by fpistm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Oct 3, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 12, 2017

Uh oh!

ghost commented Oct 17, 2017

Uh oh!

ghost commented Nov 7, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Nov 8, 2017

Uh oh!

cparata left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fpistm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost commented Oct 2, 2017 •

edited by ghost

Loading

VVESTM Oct 2, 2017 •

edited by fpistm

Loading

ghost Oct 3, 2017 •

edited by ghost

Loading

ghost commented Nov 7, 2017 •

edited by ghost

Loading

cparata left a comment •

edited

Loading