From 0265515a0cb67546e830102348a252dfd89075d0 Mon Sep 17 00:00:00 2001 From: Romuald Conty Date: Thu, 5 May 2011 09:27:17 +0000 Subject: [PATCH] Abort mecanism is now implemented in driver layer: iAbortFd file descriptor array have been removed from nfc_device_t; nfc_abort_command() can now failed (return false); nfc_abort_command() now call abort_command pointer from drivers; pn532_uart and arygon drivers use a pipe-based mecanism (similar from previous one); pn53x_usb driver use a boolean flag-based mecanism (the previous one does not work as expected); pn53x_usb now print smarter messages on error at usb connection; pn53x_usb now handle a strange case: sometimes, the first sent command is not ACKed by PN53x USB device, a dummy command is now sent. --- include/nfc/nfc-types.h | 1 - include/nfc/nfc.h | 2 +- libnfc/drivers/acr122.c | 14 ++++---- libnfc/drivers/arygon.c | 26 +++++++++++++- libnfc/drivers/pn532_uart.c | 25 ++++++++++++- libnfc/drivers/pn53x_usb.c | 71 ++++++++++++++++++++----------------- libnfc/nfc-internal.h | 2 ++ libnfc/nfc.c | 13 ++----- 8 files changed, 102 insertions(+), 52 deletions(-) diff --git a/include/nfc/nfc-types.h b/include/nfc/nfc-types.h index 2bddb7e..5ae639d 100644 --- a/include/nfc/nfc-types.h +++ b/include/nfc/nfc-types.h @@ -66,7 +66,6 @@ typedef struct { * +----------- Driver-level general error (common to all drivers) */ int iLastError; - int iAbortFds[2]; } nfc_device_t; /** diff --git a/include/nfc/nfc.h b/include/nfc/nfc.h index 0f42e36..2e37667 100644 --- a/include/nfc/nfc.h +++ b/include/nfc/nfc.h @@ -63,7 +63,7 @@ extern "C" { /* NFC Device/Hardware manipulation */ NFC_EXPORT nfc_device_t *nfc_connect (nfc_device_desc_t * pndd); NFC_EXPORT void nfc_disconnect (nfc_device_t * pnd); - NFC_EXPORT void nfc_abort_command (nfc_device_t * pnd); + NFC_EXPORT bool nfc_abort_command (nfc_device_t * pnd); NFC_EXPORT void nfc_list_devices (nfc_device_desc_t pnddDevices[], size_t szDevices, size_t * pszDeviceFound); NFC_EXPORT bool nfc_configure (nfc_device_t * pnd, const nfc_device_option_t ndo, const bool bEnable); diff --git a/libnfc/drivers/acr122.c b/libnfc/drivers/acr122.c index fc5c00b..66fbd62 100644 --- a/libnfc/drivers/acr122.c +++ b/libnfc/drivers/acr122.c @@ -409,7 +409,7 @@ const struct nfc_driver_t acr122_driver = { .initiator_init = pn53x_initiator_init, .initiator_select_passive_target = pn53x_initiator_select_passive_target, - .initiator_poll_targets = NULL, + .initiator_poll_targets = pn53x_initiator_poll_targets, .initiator_select_dep_target = pn53x_initiator_select_dep_target, .initiator_deselect_target = pn53x_initiator_deselect_target, .initiator_transceive_bytes = pn53x_initiator_transceive_bytes, @@ -417,12 +417,14 @@ const struct nfc_driver_t acr122_driver = { .initiator_transceive_bytes_timed = pn53x_initiator_transceive_bytes_timed, .initiator_transceive_bits_timed = pn53x_initiator_transceive_bits_timed, - .target_init = NULL, - .target_send_bytes = NULL, - .target_receive_bytes = NULL, - .target_send_bits = NULL, - .target_receive_bits = NULL, + .target_init = pn53x_target_init, + .target_send_bytes = pn53x_target_send_bytes, + .target_receive_bytes = pn53x_target_receive_bytes, + .target_send_bits = pn53x_target_send_bits, + .target_receive_bits = pn53x_target_receive_bits, .configure = pn53x_configure, + + .abort_command = NULL, }; diff --git a/libnfc/drivers/arygon.c b/libnfc/drivers/arygon.c index 5cc6912..45a5f90 100644 --- a/libnfc/drivers/arygon.c +++ b/libnfc/drivers/arygon.c @@ -36,6 +36,7 @@ #include #include #include +#include #include @@ -71,6 +72,7 @@ const struct pn53x_io arygon_tama_io; struct arygon_data { serial_port port; + int iAbortFds[2]; }; static const byte_t arygon_error_none[] = "FF000000\x0d\x0a"; @@ -184,6 +186,9 @@ arygon_connect (const nfc_device_desc_t * pndd) return NULL; } + // pipe-based abort mecanism + pipe (DRIVER_DATA (pnd)->iAbortFds); + char arygon_firmware_version[10]; arygon_firmware (pnd, arygon_firmware_version); char *pcName; @@ -198,7 +203,13 @@ arygon_connect (const nfc_device_desc_t * pndd) void arygon_disconnect (nfc_device_t * pnd) { + // Release UART port uart_close (DRIVER_DATA (pnd)->port); + + // Release file descriptors used for abort mecanism + close (DRIVER_DATA (pnd)->iAbortFds[0]); + close (DRIVER_DATA (pnd)->iAbortFds[1]); + nfc_device_free (pnd); } @@ -276,7 +287,7 @@ arygon_tama_receive (nfc_device_t * pnd, byte_t * pbtData, const size_t szDataLe case TgInitAsTarget: case TgGetData: case InJumpForDEP: - abort_fd = pnd->iAbortFds[1]; + abort_fd = DRIVER_DATA (pnd)->iAbortFds[1]; break; default: break; @@ -440,6 +451,17 @@ arygon_reset_tama (nfc_device_t * pnd) return true; } +bool +arygon_abort_command (nfc_device_t * pnd) +{ + if (pnd) { + close (DRIVER_DATA (pnd)->iAbortFds[0]); + pipe (DRIVER_DATA (pnd)->iAbortFds); + } + return true; +} + + const struct pn53x_io arygon_tama_io = { .send = arygon_tama_send, .receive = arygon_tama_receive, @@ -469,5 +491,7 @@ const struct nfc_driver_t arygon_driver = { .target_receive_bits = pn53x_target_receive_bits, .configure = pn53x_configure, + + .abort_command = arygon_abort_command, }; diff --git a/libnfc/drivers/pn532_uart.c b/libnfc/drivers/pn532_uart.c index 1de58a7..0cca5b3 100644 --- a/libnfc/drivers/pn532_uart.c +++ b/libnfc/drivers/pn532_uart.c @@ -33,6 +33,7 @@ #include #include +#include #include @@ -55,6 +56,7 @@ const struct pn53x_io pn532_uart_io; struct pn532_uart_data { serial_port port; + int iAbortFds[2]; }; #define DRIVER_DATA(pnd) ((struct pn532_uart_data*)(pnd->driver_data)) @@ -171,6 +173,9 @@ pn532_uart_connect (const nfc_device_desc_t * pndd) return NULL; } + // pipe-based abort mecanism + pipe (DRIVER_DATA (pnd)->iAbortFds); + pn53x_init(pnd); return pnd; } @@ -178,7 +183,13 @@ pn532_uart_connect (const nfc_device_desc_t * pndd) void pn532_uart_disconnect (nfc_device_t * pnd) { + // Release UART port uart_close (DRIVER_DATA(pnd)->port); + + // Release file descriptors used for abort mecanism + close (DRIVER_DATA (pnd)->iAbortFds[0]); + close (DRIVER_DATA (pnd)->iAbortFds[1]); + nfc_device_free (pnd); } @@ -263,7 +274,7 @@ pn532_uart_receive (nfc_device_t * pnd, byte_t * pbtData, const size_t szDataLen case InJumpForDEP: case TgInitAsTarget: case TgGetData: - abort_fd = pnd->iAbortFds[1]; + abort_fd = DRIVER_DATA (pnd)->iAbortFds[1]; break; default: break; @@ -390,6 +401,16 @@ pn532_uart_ack (nfc_device_t * pnd) return (0 == uart_send (DRIVER_DATA(pnd)->port, ack_frame, sizeof (ack_frame))) ? 0 : -1; } +bool +pn532_uart_abort_command (nfc_device_t * pnd) +{ + if (pnd) { + close (DRIVER_DATA (pnd)->iAbortFds[0]); + pipe (DRIVER_DATA (pnd)->iAbortFds); + } + return true; +} + const struct pn53x_io pn532_uart_io = { .send = pn532_uart_send, .receive = pn532_uart_receive, @@ -419,5 +440,7 @@ const struct nfc_driver_t pn532_uart_driver = { .target_receive_bits = pn53x_target_receive_bits, .configure = pn53x_configure, + + .abort_command = pn532_uart_abort_command, }; diff --git a/libnfc/drivers/pn53x_usb.c b/libnfc/drivers/pn53x_usb.c index fa8930c..612617b 100644 --- a/libnfc/drivers/pn53x_usb.c +++ b/libnfc/drivers/pn53x_usb.c @@ -66,6 +66,7 @@ struct pn53x_usb_data { uint32_t uiEndPointIn; uint32_t uiEndPointOut; uint32_t uiMaxPacketSize; + volatile bool abort_flag; }; const struct pn53x_io pn53x_usb_io; @@ -261,17 +262,23 @@ pn53x_usb_connect (const nfc_device_desc_t *pndd) if (uiBusIndex == 0) { // Open the USB device data.pudh = usb_open (dev); - + // Retrieve end points pn53x_usb_get_end_points (dev, &data); - if (usb_set_configuration (data.pudh, 1) < 0) { - ERR ("Unable to set USB configuration, please check USB permissions for device %04x:%04x", dev->descriptor.idVendor, dev->descriptor.idProduct); + // Set configuration + int res = usb_set_configuration (data.pudh, 1); + if (res < 0) { + ERR ("Unable to set USB configuration (%s)", strerror (-res)); + if (EPERM == -res) { + WARN ("Please double check USB permissions for device %04x:%04x", dev->descriptor.idVendor, dev->descriptor.idProduct); + } usb_close (data.pudh); // we failed to use the specified device return NULL; } - if (usb_claim_interface (data.pudh, 0) < 0) { - DBG ("%s", "Can't claim interface"); + res = usb_claim_interface (data.pudh, 0); + if (res < 0) { + DBG ("Can't claim interface (%s)", strerror (-res)); usb_close (data.pudh); // we failed to use the specified device return NULL; @@ -317,7 +324,7 @@ pn53x_usb_connect (const nfc_device_desc_t *pndd) usb_close (data.pudh); goto error; } - + DRIVER_DATA (pnd)->abort_flag = false; return pnd; } } @@ -377,7 +384,7 @@ pn53x_usb_send (nfc_device_t * pnd, const byte_t * pbtData, const size_t szData) return false; } - byte_t abtRxBuf[6]; + byte_t abtRxBuf[PN53X_USB_BUFFER_LEN]; res = pn53x_usb_bulk_read (DRIVER_DATA (pnd), abtRxBuf, sizeof (abtRxBuf)); if (res < 0) { DBG ("usb_bulk_read failed with error %d", res); @@ -401,13 +408,14 @@ pn53x_usb_receive (nfc_device_t * pnd, byte_t * pbtData, const size_t szDataLen) { size_t len; off_t offset = 0; - int abort_fd = 0; + bool delayed_reply = false; switch (CHIP_DATA (pnd)->ui8LastCommand) { - case TgInitAsTarget: case InJumpForDEP: + case TgInitAsTarget: case TgGetData: - abort_fd = pnd->iAbortFds[1]; + DBG ("Delayed reply detected"); + delayed_reply = true; break; default: break; @@ -417,36 +425,21 @@ pn53x_usb_receive (nfc_device_t * pnd, byte_t * pbtData, const size_t szDataLen) int res; read: - res = pn53x_usb_bulk_read_ex (DRIVER_DATA (pnd), abtRxBuf, sizeof (abtRxBuf), 250); + res = pn53x_usb_bulk_read_ex (DRIVER_DATA (pnd), abtRxBuf, sizeof (abtRxBuf), delayed_reply ? 250 : 0); - if (res == -ETIMEDOUT) { - struct timeval tv = { - .tv_sec = 0, - .tv_usec = 0, - }; - - fd_set readfds; - FD_ZERO(&readfds); - FD_SET(abort_fd, &readfds); - switch (select (abort_fd + 1, &readfds, NULL, NULL, &tv)) { - case -1: - // An error occured - return false; - break; - case 0: - // Timeout - goto read; - break; - case 1: + if (delayed_reply && (res == -ETIMEDOUT)) { + if (DRIVER_DATA (pnd)->abort_flag) { + DRIVER_DATA (pnd)->abort_flag = false; pn53x_usb_ack (pnd); pnd->iLastError = DEABORT; return -1; - break; + } else { + goto read; } } if (res < 0) { - DBG ("usb_bulk_read failed with error %d", res); + DBG ("usb_bulk_read failed with error %d (%s)", res, strerror(-res)); pnd->iLastError = DEIO; // try to interrupt current device state pn53x_usb_ack(pnd); @@ -540,6 +533,11 @@ pn53x_usb_ack (nfc_device_t * pnd) bool pn53x_usb_init (nfc_device_t *pnd) { + // Sometimes PN53x USB doesn't reply ACK one the first frame, so we need to send a dummy one... + pn53x_check_communication (pnd); + // ...and we don't care about error + pnd->iLastError = 0; + if (!pn53x_init (pnd)) return false; @@ -633,6 +631,13 @@ pn53x_usb_configure (nfc_device_t * pnd, const nfc_device_option_t ndo, const bo return true; } +bool +pn53x_usb_abort_command (nfc_device_t * pnd) +{ + DRIVER_DATA (pnd)->abort_flag = true; + return true; +} + const struct pn53x_io pn53x_usb_io = { .send = pn53x_usb_send, .receive = pn53x_usb_receive, @@ -662,4 +667,6 @@ const struct nfc_driver_t pn53x_usb_driver = { .target_receive_bits = pn53x_target_receive_bits, .configure = pn53x_usb_configure, + + .abort_command = pn53x_usb_abort_command, }; diff --git a/libnfc/nfc-internal.h b/libnfc/nfc-internal.h index 7e78fde..ee427bc 100644 --- a/libnfc/nfc-internal.h +++ b/libnfc/nfc-internal.h @@ -124,6 +124,8 @@ struct nfc_driver_t { bool (*target_receive_bits) (nfc_device_t * pnd, byte_t * pbtRx, size_t * pszRxBits, byte_t * pbtRxPar); bool (*configure) (nfc_device_t * pnd, const nfc_device_option_t ndo, const bool bEnable); + + bool (*abort_command) (nfc_device_t * pnd); }; nfc_device_t *nfc_device_new (void); diff --git a/libnfc/nfc.c b/libnfc/nfc.c index 1e95e09..acc5c32 100644 --- a/libnfc/nfc.c +++ b/libnfc/nfc.c @@ -35,7 +35,6 @@ #include #include #include -#include #include @@ -138,8 +137,6 @@ nfc_connect (nfc_device_desc_t * pndd) if (!nfc_configure (pnd, NDO_ACCEPT_MULTIPLE_FRAMES, false)) return NULL; - pipe (pnd->iAbortFds); - return pnd; } else { DBG ("No device found using driver: %s", ndr->name); @@ -164,8 +161,6 @@ nfc_disconnect (nfc_device_t * pnd) nfc_initiator_deselect_target (pnd); // Disable RF field to avoid heating nfc_configure (pnd, NDO_ACTIVATE_FIELD, false); - close (pnd->iAbortFds[0]); - close (pnd->iAbortFds[1]); // Disconnect, clean up and release the device pnd->driver->disconnect (pnd); } @@ -608,13 +603,11 @@ nfc_target_init (nfc_device_t * pnd, nfc_target_t * pnt, byte_t * pbtRx, size_t HAL (target_init, pnd, pnt, pbtRx, pszRx); } -void +/* TODO Document this function */ +bool nfc_abort_command (nfc_device_t * pnd) { - if (pnd) { - close (pnd->iAbortFds[0]); - pipe (pnd->iAbortFds); - } + HAL (abort_command, pnd); } /**