From 1cd18f26271ec565429343b9c1ae5ab0808d97bb Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Mon, 31 Jan 2022 20:03:18 +0100 Subject: [PATCH 01/14] Added XLinkConnect with timeout --- include/XLink/XLink.h | 8 ++++++++ src/shared/XLinkDevice.c | 10 ++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/include/XLink/XLink.h b/include/XLink/XLink.h index d4731248..c1784b1a 100644 --- a/include/XLink/XLink.h +++ b/include/XLink/XLink.h @@ -74,6 +74,14 @@ XLinkError_t XLinkFindAllSuitableDevices(XLinkDeviceState_t state, */ XLinkError_t XLinkConnect(XLinkHandler_t* handler); +/** + * @brief Connects to specific device with a timeout, starts dispatcher and pings remote + * @param[in,out] handler - XLink communication parameters (file path name for underlying layer) + * @param[in] msTimeout – time in milliseconds after which operation times out + * @return Status code of the operation: X_LINK_SUCCESS (0) for success + */ +XLinkError_t XLinkConnectWithTimeout(XLinkHandler_t* handler, const unsigned int msTimeout); + /** * @brief Puts device into bootloader mode * @param deviceDesc - device description structure, obtained from XLinkFind* functions call diff --git a/src/shared/XLinkDevice.c b/src/shared/XLinkDevice.c index 7e33610e..bb983944 100644 --- a/src/shared/XLinkDevice.c +++ b/src/shared/XLinkDevice.c @@ -178,8 +178,14 @@ XLinkError_t XLinkFindAllSuitableDevices(XLinkDeviceState_t state, return parsePlatformError(rc); } -//Called only from app - per device + XLinkError_t XLinkConnect(XLinkHandler_t* handler) +{ + return XLinkConnectWithTimeout(handler, XLINK_NO_RW_TIMEOUT); +} + +//Called only from app - per device +XLinkError_t XLinkConnectWithTimeout(XLinkHandler_t* handler, const unsigned int msTimeout) { XLINK_RET_IF(handler == NULL); if (strnlen(handler->devicePath, MAX_PATH_LENGTH) < 2) { @@ -217,7 +223,7 @@ XLinkError_t XLinkConnect(XLinkHandler_t* handler) event.deviceHandle = link->deviceHandle; DispatcherAddEvent(EVENT_LOCAL, &event); - if (DispatcherWaitEventComplete(&link->deviceHandle, XLINK_NO_RW_TIMEOUT)) { + if (DispatcherWaitEventComplete(&link->deviceHandle, msTimeout)) { DispatcherClean(&link->deviceHandle); return X_LINK_TIMEOUT; } From 1a7d0d5aecab9feef106504b4011edbc1c2842da Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Wed, 2 Feb 2022 02:26:10 +0100 Subject: [PATCH 02/14] Added duplicate TCPIP entries filtering --- src/pc/protocols/tcpip_host.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/pc/protocols/tcpip_host.c b/src/pc/protocols/tcpip_host.c index 30539ac9..d1766dea 100644 --- a/src/pc/protocols/tcpip_host.c +++ b/src/pc/protocols/tcpip_host.c @@ -354,8 +354,29 @@ xLinkPlatformErrorCode_t tcpip_get_devices(XLinkDeviceState_t state, deviceDesc_ tcpip_close_socket(sock); + // Filter out duplicates - routing table will decide through which interface the packets will traverse + // TODO(themarpe) - properly separate interfaces. + // Either bind to interface addr, or SO_BINDTODEVICE Linux, IP_BOUND_IF macOS, and prefix interface name + int write_index = 0; + for(int i = 0; i < (int) num_devices_match; i++){ + bool duplicate = false; + for(int j = i - 1; j >= 0; j--){ + // Check if duplicate + + // TMP TMP - merge with device search improvements + // to have mxid available as well + if(strcmp(devices[i].name, devices[j].name) == 0){ + duplicate = true; + break; + } + } + if(!duplicate){ + devices[write_index] = devices[i]; + write_index++; + } + } // return total device found - *device_count = num_devices_match; + *device_count = write_index; // if at least one device matched, return OK otherwise return not found if(num_devices_match <= 0) From 0f712b456ab1c094ce86cb0a67f09e1fa23ecf6b Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Wed, 2 Feb 2022 02:50:53 +0100 Subject: [PATCH 03/14] Added additional graceful return from stuck TCP devices --- include/XLink/XLink.h | 28 +++---- include/XLink/XLinkPrivateFields.h | 4 +- src/pc/PlatformDeviceControl.c | 2 +- src/shared/XLinkData.c | 119 ++++++----------------------- src/shared/XLinkDevice.c | 86 +++++---------------- src/shared/XLinkDispatcher.c | 56 ++++---------- src/shared/XLinkDispatcherImpl.c | 31 ++++---- src/shared/XLinkPrivateFields.c | 47 ++++++++++++ 8 files changed, 139 insertions(+), 234 deletions(-) diff --git a/include/XLink/XLink.h b/include/XLink/XLink.h index c1784b1a..5e99e274 100644 --- a/include/XLink/XLink.h +++ b/include/XLink/XLink.h @@ -75,6 +75,7 @@ XLinkError_t XLinkFindAllSuitableDevices(XLinkDeviceState_t state, XLinkError_t XLinkConnect(XLinkHandler_t* handler); /** + * TODO - doesn't work well yet... * @brief Connects to specific device with a timeout, starts dispatcher and pings remote * @param[in,out] handler - XLink communication parameters (file path name for underlying layer) * @param[in] msTimeout – time in milliseconds after which operation times out @@ -122,22 +123,17 @@ XLinkError_t XLinkBootFirmware(const deviceDesc_t* deviceDesc, const char* firmw * @return Status code of the operation: X_LINK_SUCCESS (0) for success */ -XLinkError_t XLinkResetRemote(linkId_t id); +XLinkError_t XLinkResetRemote(const linkId_t id); /** * @brief Resets the remote device and close all open local handles for this device * @warning This function should be used in a host application * @param[in] id - link Id obtained from XLinkConnect in the handler parameter + * @param[in] msTimeout – time in milliseconds after which operation times out * @return Status code of the operation: X_LINK_SUCCESS (0) for success */ -XLinkError_t XLinkResetRemoteTimeout(linkId_t id, int timeoutMs); - -/** - * @brief Closes all and release all memory - * @return Status code of the operation: X_LINK_SUCCESS (0) for success - */ -XLinkError_t XLinkResetAll(); +XLinkError_t XLinkResetRemoteTimeout(const linkId_t id, const unsigned int msTimeout); /** * @brief Retrieves USB speed of certain connected device @@ -311,10 +307,10 @@ XLinkError_t XLinkReleaseData(streamId_t const streamId); * data. Should be fixed for the next release. * @param[in] streamId – stream link Id obtained from XLinkOpenStream call * @param[out] packet – structure containing output data buffer and received size - * @param[in] timeoutMs – timeout for a read operation in milliseconds + * @param[in] msTimeout – timeout for a read operation in milliseconds * @return Status code of the operation: X_LINK_SUCCESS (0) for success */ -XLinkError_t XLinkReadDataWithTimeout(streamId_t streamId, streamPacketDesc_t** packet, unsigned int timeoutMs); +XLinkError_t XLinkReadDataWithTimeout(streamId_t streamId, streamPacketDesc_t** packet, unsigned int msTimeout); /** * @brief Sends a package to initiate the writing of data to a remote stream with timeout in ms @@ -323,10 +319,10 @@ XLinkError_t XLinkReadDataWithTimeout(streamId_t streamId, streamPacketDesc_t** * @param[in] streamId – stream link Id obtained from XLinkOpenStream call * @param[in] buffer – data buffer to be transmitted * @param[in] size – size of the data to be transmitted - * @param[in] timeoutMs – timeout for a write operation in milliseconds + * @param[in] msTimeout – timeout for a write operation in milliseconds * @return Status code of the operation: X_LINK_SUCCESS (0) for success */ -XLinkError_t XLinkWriteDataWithTimeout(streamId_t streamId, const uint8_t* buffer, int size, unsigned int timeoutMs); +XLinkError_t XLinkWriteDataWithTimeout(streamId_t streamId, const uint8_t* buffer, int size, unsigned int msTimeout); // ------------------------------------ // Device streams management. End. @@ -354,8 +350,12 @@ XLinkError_t XLinkAsyncWriteData(); XLinkError_t XLinkSetDeviceOpenTimeOutMsec(unsigned int msec); XLinkError_t XLinkSetCommonTimeOutMsec(unsigned int msec); -// unsafe -XLinkError_t XLinkGetFillLevel(streamId_t const streamId, int isRemote, int* fillLevel); +/** + * Deprecated - issues + * @brief Closes all and release all memory + * @return Status code of the operation: X_LINK_SUCCESS (0) for success + */ +XLinkError_t XLinkResetAll(); #endif // __PC__ diff --git a/include/XLink/XLinkPrivateFields.h b/include/XLink/XLinkPrivateFields.h index 0e831374..68a3a894 100644 --- a/include/XLink/XLinkPrivateFields.h +++ b/include/XLink/XLinkPrivateFields.h @@ -40,8 +40,10 @@ extern sem_t pingSem; //to b used by myriad xLinkDesc_t* getLinkById(linkId_t id); xLinkDesc_t* getLink(void* fd); +xLinkDesc_t* getLinkUnsafe(void* fd); xLinkState_t getXLinkState(xLinkDesc_t* link); - +XLinkError_t getLinkUpDeviceHandleByLinkId(linkId_t const linkId, xLinkDeviceHandle_t* const out_handle); +XLinkError_t getLinkUpDeviceHandleByStreamId(streamId_t const streamId, xLinkDeviceHandle_t* const out_handle); streamId_t getStreamIdByName(xLinkDesc_t* link, const char* name); diff --git a/src/pc/PlatformDeviceControl.c b/src/pc/PlatformDeviceControl.c index 489cfcca..cbbdf9d9 100644 --- a/src/pc/PlatformDeviceControl.c +++ b/src/pc/PlatformDeviceControl.c @@ -46,7 +46,7 @@ int usbFdRead = -1; static UsbSpeed_t usb_speed_enum = X_LINK_USB_SPEED_UNKNOWN; static char mx_serial[XLINK_MAX_MX_ID_SIZE] = { 0 }; #ifdef USE_USB_VSC -static int statuswaittimeout = 5; +static const int statuswaittimeout = 5; #endif typedef struct { diff --git a/src/shared/XLinkData.c b/src/shared/XLinkData.c index f02b8a4f..73d310a9 100644 --- a/src/shared/XLinkData.c +++ b/src/shared/XLinkData.c @@ -36,9 +36,7 @@ static XLinkError_t checkEventHeader(xLinkEventHeader_t header); static float timespec_diff(struct timespec *start, struct timespec *stop); static XLinkError_t addEvent(xLinkEvent_t *event, unsigned int timeoutMs); static XLinkError_t addEventWithPerf(xLinkEvent_t *event, float* opTime, unsigned int timeoutMs); -static XLinkError_t addEventWithPerfTimeout(xLinkEvent_t *event, float* opTime, unsigned int msTimeout); static XLinkError_t getLinkByStreamId(streamId_t streamId, xLinkDesc_t** out_link); - // ------------------------------------ // Helpers declaration. End. // ------------------------------------ @@ -106,13 +104,13 @@ streamId_t XLinkOpenStream(linkId_t id, const char* name, int stream_write_size) // and on the remote side we are freeing the read buffer XLinkError_t XLinkCloseStream(streamId_t const streamId) { - xLinkDesc_t* link = NULL; - XLINK_RET_IF(getLinkByStreamId(streamId, &link)); + xLinkDeviceHandle_t deviceHandle; + XLINK_RET_IF(getLinkUpDeviceHandleByStreamId(streamId, &deviceHandle)); streamId_t streamIdOnly = EXTRACT_STREAM_ID(streamId); xLinkEvent_t event = {0}; XLINK_INIT_EVENT(event, streamIdOnly, XLINK_CLOSE_STREAM_REQ, - 0, NULL, link->deviceHandle); + 0, NULL, deviceHandle); XLINK_RET_IF(addEvent(&event, XLINK_NO_RW_TIMEOUT)); return X_LINK_SUCCESS; @@ -124,13 +122,13 @@ XLinkError_t XLinkWriteData(streamId_t const streamId, const uint8_t* buffer, XLINK_RET_IF(buffer == NULL); float opTime = 0.0f; - xLinkDesc_t* link = NULL; - XLINK_RET_IF(getLinkByStreamId(streamId, &link)); + xLinkDeviceHandle_t deviceHandle; + XLINK_RET_IF(getLinkUpDeviceHandleByStreamId(streamId, &deviceHandle)); streamId_t streamIdOnly = EXTRACT_STREAM_ID(streamId); xLinkEvent_t event = {0}; XLINK_INIT_EVENT(event, streamIdOnly, XLINK_WRITE_REQ, - size,(void*)buffer, link->deviceHandle); + size,(void*)buffer, deviceHandle); XLINK_RET_IF(addEventWithPerf(&event, &opTime, XLINK_NO_RW_TIMEOUT)); @@ -147,13 +145,13 @@ XLinkError_t XLinkReadData(streamId_t const streamId, streamPacketDesc_t** packe XLINK_RET_IF(packet == NULL); float opTime = 0.0f; - xLinkDesc_t* link = NULL; - XLINK_RET_IF(getLinkByStreamId(streamId, &link)); + xLinkDeviceHandle_t deviceHandle; + XLINK_RET_IF(getLinkUpDeviceHandleByStreamId(streamId, &deviceHandle)); streamId_t streamIdOnly = EXTRACT_STREAM_ID(streamId); xLinkEvent_t event = {0}; XLINK_INIT_EVENT(event, streamIdOnly, XLINK_READ_REQ, - 0, NULL, link->deviceHandle); + 0, NULL, deviceHandle); XLINK_RET_IF(addEventWithPerf(&event, &opTime, XLINK_NO_RW_TIMEOUT)); @@ -176,13 +174,13 @@ XLinkError_t XLinkWriteDataWithTimeout(streamId_t const streamId, const uint8_t* XLINK_RET_IF(buffer == NULL); float opTime = 0.0f; - xLinkDesc_t* link = NULL; - XLINK_RET_IF(getLinkByStreamId(streamId, &link)); + xLinkDeviceHandle_t deviceHandle; + XLINK_RET_IF(getLinkUpDeviceHandleByStreamId(streamId, &deviceHandle)); streamId_t streamIdOnly = EXTRACT_STREAM_ID(streamId); xLinkEvent_t event = {0}; XLINK_INIT_EVENT(event, streamIdOnly, XLINK_WRITE_REQ, - size,(void*)buffer, link->deviceHandle); + size,(void*)buffer, deviceHandle); mvLog(MVLOG_WARN,"XLinkWriteDataWithTimeout is not fully supported yet. The XLinkWriteData method is called instead. Desired timeout = %d\n", timeoutMs); XLINK_RET_IF_FAIL(addEventWithPerf(&event, &opTime, timeoutMs)); @@ -200,13 +198,13 @@ XLinkError_t XLinkReadDataWithTimeout(streamId_t streamId, streamPacketDesc_t** XLINK_RET_IF(packet == NULL); float opTime = 0.0f; - xLinkDesc_t* link = NULL; - XLINK_RET_IF(getLinkByStreamId(streamId, &link)); + xLinkDeviceHandle_t deviceHandle; + XLINK_RET_IF(getLinkUpDeviceHandleByStreamId(streamId, &deviceHandle)); streamId_t streamIdOnly = EXTRACT_STREAM_ID(streamId); xLinkEvent_t event = {0}; XLINK_INIT_EVENT(event, streamId, XLINK_READ_REQ, - 0, NULL, link->deviceHandle); + 0, NULL, deviceHandle); XLINK_RET_IF_FAIL(addEventWithPerf(&event, &opTime, timeoutMs)); @@ -228,13 +226,13 @@ XLinkError_t XLinkReadMoveData(streamId_t const streamId, streamPacketDesc_t* co XLINK_RET_IF(packet == NULL); float opTime = 0; - xLinkDesc_t *link = NULL; - XLINK_RET_IF(getLinkByStreamId(streamId, &link)); + xLinkDeviceHandle_t deviceHandle; + XLINK_RET_IF(getLinkUpDeviceHandleByStreamId(streamId, &deviceHandle)); streamId_t streamIdOnly = EXTRACT_STREAM_ID(streamId); xLinkEvent_t event = {0}; XLINK_INIT_EVENT(event, streamIdOnly, XLINK_READ_REQ, - 0, NULL, link->deviceHandle); + 0, NULL, deviceHandle); event.header.flags.bitField.moveSemantic = 1; XLINK_RET_IF(addEventWithPerf(&event, &opTime, XLINK_NO_RW_TIMEOUT)); @@ -269,16 +267,16 @@ XLinkError_t XLinkReadMoveDataWithTimeout(streamId_t const streamId, streamPacke XLINK_RET_IF(packet == NULL); float opTime = 0; - xLinkDesc_t *link = NULL; - XLINK_RET_IF(getLinkByStreamId(streamId, &link)); + xLinkDeviceHandle_t deviceHandle; + XLINK_RET_IF(getLinkUpDeviceHandleByStreamId(streamId, &deviceHandle)); streamId_t streamIdOnly = EXTRACT_STREAM_ID(streamId); xLinkEvent_t event = {0}; XLINK_INIT_EVENT(event, streamIdOnly, XLINK_READ_REQ, - 0, NULL, link->deviceHandle); + 0, NULL, deviceHandle); event.header.flags.bitField.moveSemantic = 1; - const XLinkError_t rc = addEventWithPerfTimeout(&event, &opTime, msTimeout); + const XLinkError_t rc = addEventWithPerf(&event, &opTime, msTimeout); if(rc == X_LINK_TIMEOUT) return rc; else XLINK_RET_IF(rc); @@ -314,13 +312,13 @@ void XLinkDeallocateMoveData(void* const data, const uint32_t length) { XLinkError_t XLinkReleaseData(streamId_t const streamId) { - xLinkDesc_t* link = NULL; - XLINK_RET_IF(getLinkByStreamId(streamId, &link)); + xLinkDeviceHandle_t deviceHandle; + XLINK_RET_IF(getLinkUpDeviceHandleByStreamId(streamId, &deviceHandle)); streamId_t streamIdOnly = EXTRACT_STREAM_ID(streamId); xLinkEvent_t event = {0}; XLINK_INIT_EVENT(event, streamIdOnly, XLINK_READ_REL_REQ, - 0, NULL, link->deviceHandle); + 0, NULL, deviceHandle); XLINK_RET_IF(addEvent(&event, XLINK_NO_RW_TIMEOUT)); @@ -342,27 +340,6 @@ XLinkError_t XLinkReleaseSpecificData(streamId_t streamId, streamPacketDesc_t* p return X_LINK_SUCCESS; } -XLinkError_t XLinkGetFillLevel(streamId_t const streamId, int isRemote, int* fillLevel) -{ - xLinkDesc_t* link = NULL; - XLINK_RET_IF(getLinkByStreamId(streamId, &link)); - streamId_t streamIdOnly = EXTRACT_STREAM_ID(streamId); - - streamDesc_t* stream = - getStreamById(link->deviceHandle.xLinkFD, streamIdOnly); - ASSERT_XLINK(stream); - - if (isRemote) { - *fillLevel = stream->remoteFillLevel; - } - else { - *fillLevel = stream->localFillLevel; - } - - releaseStream(stream); - return X_LINK_SUCCESS; -} - // ------------------------------------ // Helpers declaration. Begin. // ------------------------------------ @@ -468,52 +445,6 @@ XLinkError_t addEventWithPerf(xLinkEvent_t *event, float* opTime, unsigned int t return X_LINK_SUCCESS; } -XLinkError_t addEventTimeout(xLinkEvent_t *event, struct timespec abstime) -{ - ASSERT_XLINK(event); - - xLinkEvent_t* ev = DispatcherAddEvent(EVENT_LOCAL, event); - if(ev == NULL) { - mvLog(MVLOG_ERROR, "Dispatcher failed on adding event. type: %s, id: %d, stream name: %s\n", - TypeToStr(event->header.type), event->header.id, event->header.streamName); - return X_LINK_ERROR; - } - - if (DispatcherWaitEventCompleteTimeout(&event->deviceHandle, abstime)) { - return X_LINK_TIMEOUT; - } - - XLINK_RET_ERR_IF( - event->header.flags.bitField.ack != 1, - X_LINK_COMMUNICATION_FAIL); - - return X_LINK_SUCCESS; -} - -XLinkError_t addEventWithPerfTimeout(xLinkEvent_t *event, float* opTime, unsigned int msTimeout) -{ - ASSERT_XLINK(opTime); - - struct timespec start, end; - clock_gettime(CLOCK_REALTIME, &start); - - struct timespec absTimeout = start; - int64_t sec = msTimeout / 1000; - absTimeout.tv_sec += sec; - absTimeout.tv_nsec += (long)((msTimeout - (sec * 1000)) * 1000000); - int64_t secOver = absTimeout.tv_nsec / 1000000000; - absTimeout.tv_nsec -= (long)(secOver * 1000000000); - absTimeout.tv_sec += secOver; - - int rc = addEventTimeout(event, absTimeout); - if(rc != X_LINK_SUCCESS) return rc; - - clock_gettime(CLOCK_REALTIME, &end); - *opTime = timespec_diff(&start, &end); - - return X_LINK_SUCCESS; -} - static XLinkError_t getLinkByStreamId(streamId_t streamId, xLinkDesc_t** out_link) { ASSERT_XLINK(out_link != NULL); diff --git a/src/shared/XLinkDevice.c b/src/shared/XLinkDevice.c index bb983944..b885f037 100644 --- a/src/shared/XLinkDevice.c +++ b/src/shared/XLinkDevice.c @@ -178,7 +178,7 @@ XLinkError_t XLinkFindAllSuitableDevices(XLinkDeviceState_t state, return parsePlatformError(rc); } - +//Called only from app - per device XLinkError_t XLinkConnect(XLinkHandler_t* handler) { return XLinkConnectWithTimeout(handler, XLINK_NO_RW_TIMEOUT); @@ -224,7 +224,7 @@ XLinkError_t XLinkConnectWithTimeout(XLinkHandler_t* handler, const unsigned int DispatcherAddEvent(EVENT_LOCAL, &event); if (DispatcherWaitEventComplete(&link->deviceHandle, msTimeout)) { - DispatcherClean(&link->deviceHandle); + DispatcherDeviceFdDown(&link->deviceHandle); return X_LINK_TIMEOUT; } @@ -286,88 +286,40 @@ XLinkError_t XLinkBootFirmware(const deviceDesc_t* deviceDesc, const char* firmw return X_LINK_COMMUNICATION_FAIL; } -XLinkError_t XLinkResetRemote(linkId_t id) +XLinkError_t XLinkResetRemote(const linkId_t id) { - xLinkDesc_t* link = getLinkById(id); - XLINK_RET_IF(link == NULL); - - if (getXLinkState(link) != XLINK_UP) { - mvLog(MVLOG_WARN, "Link is down, close connection to device without reset"); - XLinkPlatformCloseRemote(&link->deviceHandle); - return X_LINK_COMMUNICATION_NOT_OPEN; - } - - // Add event to reset device. After sending it, dispatcher will close fd link - xLinkEvent_t event = {0}; - event.header.type = XLINK_RESET_REQ; - event.deviceHandle = link->deviceHandle; - mvLog(MVLOG_DEBUG, "sending reset remote event\n"); - DispatcherAddEvent(EVENT_LOCAL, &event); - XLINK_RET_ERR_IF(DispatcherWaitEventComplete(&link->deviceHandle, XLINK_NO_RW_TIMEOUT), - X_LINK_TIMEOUT); - - int rc; - while(((rc = XLink_sem_wait(&link->dispatcherClosedSem)) == -1) && errno == EINTR) - continue; - if(rc) { - mvLog(MVLOG_ERROR,"can't wait dispatcherClosedSem\n"); - return X_LINK_ERROR; - } - - return X_LINK_SUCCESS; + return XLinkResetRemoteTimeout(id, XLINK_NO_RW_TIMEOUT); } - -XLinkError_t XLinkResetRemoteTimeout(linkId_t id, int timeoutMs) +XLinkError_t XLinkResetRemoteTimeout(const linkId_t id, const unsigned int msTimeout) { - xLinkDesc_t* link = getLinkById(id); - XLINK_RET_IF(link == NULL); - - if (getXLinkState(link) != XLINK_UP) { - mvLog(MVLOG_WARN, "Link is down, close connection to device without reset"); - XLinkPlatformCloseRemote(&link->deviceHandle); - return X_LINK_COMMUNICATION_NOT_OPEN; - } + xLinkDeviceHandle_t deviceHandle; + XLINK_RET_IF(getLinkUpDeviceHandleByLinkId(id, &deviceHandle)); // Add event to reset device. After sending it, dispatcher will close fd link xLinkEvent_t event = {0}; event.header.type = XLINK_RESET_REQ; - event.deviceHandle = link->deviceHandle; + event.deviceHandle = deviceHandle; mvLog(MVLOG_DEBUG, "sending reset remote event\n"); + DispatcherAddEvent(EVENT_LOCAL, &event); + XLinkError_t ret = DispatcherWaitEventComplete(&deviceHandle, msTimeout); - struct timespec start; - clock_gettime(CLOCK_REALTIME, &start); - - struct timespec absTimeout = start; - int64_t sec = timeoutMs / 1000; - absTimeout.tv_sec += sec; - absTimeout.tv_nsec += (long)((timeoutMs - (sec * 1000)) * 1000000); - int64_t secOver = absTimeout.tv_nsec / 1000000000; - absTimeout.tv_nsec -= (long)(secOver * 1000000000); - absTimeout.tv_sec += secOver; - - xLinkEvent_t* ev = DispatcherAddEvent(EVENT_LOCAL, &event); - if(ev == NULL) { - mvLog(MVLOG_ERROR, "Dispatcher failed on adding event. type: %s, id: %d, stream name: %s\n", - TypeToStr(event.header.type), event.header.id, event.header.streamName); - return X_LINK_ERROR; - } - - XLinkError_t ret = DispatcherWaitEventCompleteTimeout(&link->deviceHandle, absTimeout); if(ret != X_LINK_SUCCESS){ // Closing device link unblocks any blocked events // Afterwards the dispatcher can properly cleanup in its own thread - DispatcherDeviceFdDown(&link->deviceHandle); + DispatcherDeviceFdDown(&deviceHandle); } - // Wait for dispatcher to be closed - if(XLink_sem_wait(&link->dispatcherClosedSem)) { - mvLog(MVLOG_ERROR,"can't wait dispatcherClosedSem\n"); - return X_LINK_ERROR; - } + // TMP TMP - investigate if not waiting for the dispatcher to shutdown is ok + // int rc; + // while(((rc = XLink_sem_wait(&link->dispatcherClosedSem)) == -1) && errno == EINTR) + // continue; + // if(rc) { + // mvLog(MVLOG_ERROR,"can't wait dispatcherClosedSem\n"); + // return X_LINK_ERROR; + // } return ret; - } XLinkError_t XLinkResetAll() diff --git a/src/shared/XLinkDispatcher.c b/src/shared/XLinkDispatcher.c index dff69a2c..fd636fa4 100644 --- a/src/shared/XLinkDispatcher.c +++ b/src/shared/XLinkDispatcher.c @@ -118,6 +118,7 @@ int numSchedulers; xLinkSchedulerState_t schedulerState[MAX_SCHEDULERS]; sem_t addSchedulerSem; +static pthread_mutex_t unique_id_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t clean_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t reset_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t num_schedulers_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -444,9 +445,8 @@ int DispatcherWaitEventComplete(xLinkDeviceHandle_t *deviceHandle, unsigned int while(((rc = XLink_sem_wait(id)) == -1) && errno == EINTR) continue; if (id == NULL || rc) { - // Calling non-thread safe dispatcherReset from external thread - // TODO - investigate further and resolve - dispatcherReset(curr); + // graceful link shutdown instead + dispatcherDeviceFdDown(curr); } } #endif @@ -454,43 +454,6 @@ int DispatcherWaitEventComplete(xLinkDeviceHandle_t *deviceHandle, unsigned int return rc; } -int DispatcherWaitEventCompleteTimeout(xLinkDeviceHandle_t *deviceHandle, struct timespec abstime) -{ - xLinkSchedulerState_t* curr = findCorrespondingScheduler(deviceHandle->xLinkFD); - ASSERT_XLINK(curr != NULL); - - XLink_sem_t* id = getSem(pthread_self(), curr); - if (id == NULL) { - return -1; - } - - int rc = XLink_sem_timedwait(id, &abstime); - int err = errno; - -#ifdef __PC__ - if (rc) { - if(err == ETIMEDOUT){ - return X_LINK_TIMEOUT; - } else { - xLinkEvent_t event = {0}; - event.header.type = XLINK_RESET_REQ; - event.deviceHandle = *deviceHandle; - mvLog(MVLOG_ERROR,"waiting is timeout, sending reset remote event"); - DispatcherAddEvent(EVENT_LOCAL, &event); - id = getSem(pthread_self(), curr); - if (id == NULL || XLink_sem_wait(id)) { - // Calling non-thread safe dispatcherReset from external thread - // TODO - investigate further and resolve - dispatcherReset(curr); - } - } - } -#endif - - return rc; -} - - char* TypeToStr(int type) { switch(type) @@ -829,8 +792,17 @@ static void postAndMarkEventServed(xLinkEventPriv_t *event) static int createUniqueID() { - static int id = 0xa; - return id++; + static eventId_t id = 0xa; + eventId_t idCopy = 0; + XLINK_RET_ERR_IF(pthread_mutex_lock(&unique_id_mutex) != 0, -1); + id++; + if(id >= INT32_MAX){ + id = 0xa; + } + idCopy = id; + XLINK_RET_ERR_IF(pthread_mutex_unlock(&unique_id_mutex) != 0, -1); + + return idCopy; } int findAvailableScheduler() diff --git a/src/shared/XLinkDispatcherImpl.c b/src/shared/XLinkDispatcherImpl.c index 05c3a46a..50ca7812 100644 --- a/src/shared/XLinkDispatcherImpl.c +++ b/src/shared/XLinkDispatcherImpl.c @@ -70,31 +70,32 @@ int dispatcherEventSend(xLinkEvent_t *event) } int dispatcherEventReceive(xLinkEvent_t* event){ - static xLinkEvent_t prevEvent = {0}; + // static xLinkEvent_t prevEvent = {0}; int rc = XLinkPlatformRead(&event->deviceHandle, &event->header, sizeof(event->header)); - mvLog(MVLOG_DEBUG,"Incoming event %p: %s %d %p prevEvent: %s %d %p\n", - event, - TypeToStr(event->header.type), - (int)event->header.id, - event->deviceHandle.xLinkFD, - TypeToStr(prevEvent.header.type), - (int)prevEvent.header.id, - prevEvent.deviceHandle.xLinkFD); + // mvLog(MVLOG_DEBUG,"Incoming event %p: %s %d %p prevEvent: %s %d %p\n", + // event, + // TypeToStr(event->header.type), + // (int)event->header.id, + // event->deviceHandle.xLinkFD, + // TypeToStr(prevEvent.header.type), + // (int)prevEvent.header.id, + // prevEvent.deviceHandle.xLinkFD); if(rc < 0) { mvLog(MVLOG_DEBUG,"%s() Read failed %d\n", __func__, (int)rc); return rc; } - if (prevEvent.header.id == event->header.id && - prevEvent.header.type == event->header.type && - prevEvent.deviceHandle.xLinkFD == event->deviceHandle.xLinkFD) { - mvLog(MVLOG_FATAL,"Duplicate id detected. \n"); - } + // TODO - reimplement duplicate ID detection + // if (prevEvent.header.id == event->header.id && + // prevEvent.header.type == event->header.type && + // prevEvent.deviceHandle.xLinkFD == event->deviceHandle.xLinkFD) { + // mvLog(MVLOG_FATAL,"Duplicate id detected. \n"); + // } + // prevEvent = *event; - prevEvent = *event; return handleIncomingEvent(event); } diff --git a/src/shared/XLinkPrivateFields.c b/src/shared/XLinkPrivateFields.c index 064b8a0d..312e9596 100644 --- a/src/shared/XLinkPrivateFields.c +++ b/src/shared/XLinkPrivateFields.c @@ -34,6 +34,40 @@ xLinkDesc_t* getLinkById(linkId_t id) return NULL; } +XLinkError_t getLinkUpDeviceHandleByStreamId(streamId_t const streamId, xLinkDeviceHandle_t* const out_handle) { + ASSERT_XLINK(out_handle != NULL); + + linkId_t id = EXTRACT_LINK_ID(streamId); + return getLinkUpDeviceHandleByLinkId(id, out_handle); +} + +XLinkError_t getLinkUpDeviceHandleByLinkId(linkId_t id, xLinkDeviceHandle_t* const out_handle) +{ + ASSERT_XLINK(out_handle); + XLINK_RET_ERR_IF(pthread_mutex_lock(&availableXLinksMutex) != 0, X_LINK_ERROR); + + // Error if no valid id found + XLinkError_t ret = X_LINK_ERROR; + for (int i = 0; i < MAX_LINKS; i++) { + if (availableXLinks[i].id == id) { + // Copy handle out before unlocking the mutex + *out_handle = availableXLinks[i].deviceHandle; + // Check if state is up + if(availableXLinks[i].peerState == XLINK_UP){ + ret = X_LINK_SUCCESS; + } else { + ret = X_LINK_COMMUNICATION_NOT_OPEN; + } + // Exit the loop + break; + } + } + + XLINK_RET_ERR_IF(pthread_mutex_unlock(&availableXLinksMutex) != 0, X_LINK_ERROR); + // Return success/error status + return ret; +} + xLinkDesc_t* getLink(void* fd) { @@ -51,6 +85,19 @@ xLinkDesc_t* getLink(void* fd) return NULL; } +xLinkDesc_t* getLinkUnsafe(void* fd) +{ + + int i; + for (i = 0; i < MAX_LINKS; i++) { + if (availableXLinks[i].deviceHandle.xLinkFD == fd) { + return &availableXLinks[i]; + } + } + + return NULL; +} + streamId_t getStreamIdByName(xLinkDesc_t* link, const char* name) { streamDesc_t* stream = getStreamByName(link, name); From 788b97039bbe118ccef2c0e79c120cd7e278444c Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Wed, 2 Feb 2022 04:00:32 +0100 Subject: [PATCH 04/14] Fixed a bug of closing the link --- src/shared/XLinkDispatcher.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shared/XLinkDispatcher.c b/src/shared/XLinkDispatcher.c index fd636fa4..8ec0b11e 100644 --- a/src/shared/XLinkDispatcher.c +++ b/src/shared/XLinkDispatcher.c @@ -237,6 +237,8 @@ XLinkError_t DispatcherStart(xLinkDeviceHandle_t *deviceHandle) schedulerState[idx].resetXLink = 0; schedulerState[idx].dispatcherLinkDown = 0; + schedulerState[idx].dispatcherDeviceFdDown = 0; + schedulerState[idx].deviceHandle = *deviceHandle; schedulerState[idx].schedulerId = idx; From d23f27e53069b6f8d1239d29816133bb6308dc37 Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Wed, 2 Feb 2022 05:18:00 +0100 Subject: [PATCH 05/14] WIP: Leaky "unique" FD for TCP protocol --- src/pc/PlatformData.c | 12 +++++++++--- src/pc/PlatformDeviceControl.c | 9 +++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/pc/PlatformData.c b/src/pc/PlatformData.c index 76029189..4ae8164c 100644 --- a/src/pc/PlatformData.c +++ b/src/pc/PlatformData.c @@ -374,8 +374,11 @@ static int tcpipPlatformRead(void *fd, void *data, int size) while(nread < size) { - TCPIP_SOCKET sock = (TCPIP_SOCKET) fd; - rc = recv((TCPIP_SOCKET)fd, &((char*)data)[nread], size - nread, 0); + // TMP TMP - leaky test + TCPIP_SOCKET sock = *((TCPIP_SOCKET*)fd); + // TCPIP_SOCKET sock = (TCPIP_SOCKET) fd; + + rc = recv((TCPIP_SOCKET)sock, &((char*)data)[nread], size - nread, 0); if(rc <= 0) { return -1; @@ -407,7 +410,10 @@ static int tcpipPlatformWrite(void *fd, void *data, int size) flags = MSG_NOSIGNAL; #endif - TCPIP_SOCKET sock = (TCPIP_SOCKET) fd; + // TMP TMP - leaky test + TCPIP_SOCKET sock = *((TCPIP_SOCKET*)fd); + // TCPIP_SOCKET sock = (TCPIP_SOCKET) fd; + rc = send(sock, &((char*)data)[byteCount], size - byteCount, flags); if(rc <= 0) { diff --git a/src/pc/PlatformDeviceControl.c b/src/pc/PlatformDeviceControl.c index cbbdf9d9..bbf58ee5 100644 --- a/src/pc/PlatformDeviceControl.c +++ b/src/pc/PlatformDeviceControl.c @@ -694,7 +694,10 @@ int tcpipPlatformConnect(const char *devPathRead, const char *devPathWrite, void return -1; } - *((TCPIP_SOCKET*)fd) = sock; + // TMP TMP - leaky "key" for FD + *fd = malloc(sizeof(TCPIP_SOCKET)); + *((TCPIP_SOCKET*)*fd) = sock; + #endif return 0; } @@ -778,7 +781,9 @@ int tcpipPlatformClose(void *fd) return status; #else - intptr_t sockfd = (intptr_t)fd; + // intptr_t sockfd = (intptr_t)fd; + // TMP TMP - leaky malloc test + TCPIP_SOCKET sockfd = *((TCPIP_SOCKET*)fd); if(sockfd != -1) { status = shutdown(sockfd, SHUT_RDWR); From 3a8f1b5f46e765d6b0508440663de3521d72b242 Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Tue, 8 Mar 2022 01:22:08 +0100 Subject: [PATCH 06/14] Reduced modifications --- include/XLink/XLink.h | 4 ++-- src/pc/protocols/tcpip_host.c | 2 +- src/shared/XLinkDevice.c | 35 +++++++++++++++++++------------- src/shared/XLinkDispatcher.c | 8 ++++++-- src/shared/XLinkDispatcherImpl.c | 2 +- 5 files changed, 31 insertions(+), 20 deletions(-) diff --git a/include/XLink/XLink.h b/include/XLink/XLink.h index 5e99e274..679d4ef9 100644 --- a/include/XLink/XLink.h +++ b/include/XLink/XLink.h @@ -75,13 +75,13 @@ XLinkError_t XLinkFindAllSuitableDevices(XLinkDeviceState_t state, XLinkError_t XLinkConnect(XLinkHandler_t* handler); /** - * TODO - doesn't work well yet... + * TODO(themarpe) - doesn't work well yet... * @brief Connects to specific device with a timeout, starts dispatcher and pings remote * @param[in,out] handler - XLink communication parameters (file path name for underlying layer) * @param[in] msTimeout – time in milliseconds after which operation times out * @return Status code of the operation: X_LINK_SUCCESS (0) for success - */ XLinkError_t XLinkConnectWithTimeout(XLinkHandler_t* handler, const unsigned int msTimeout); +*/ /** * @brief Puts device into bootloader mode diff --git a/src/pc/protocols/tcpip_host.c b/src/pc/protocols/tcpip_host.c index d1766dea..6ce966df 100644 --- a/src/pc/protocols/tcpip_host.c +++ b/src/pc/protocols/tcpip_host.c @@ -363,7 +363,7 @@ xLinkPlatformErrorCode_t tcpip_get_devices(XLinkDeviceState_t state, deviceDesc_ for(int j = i - 1; j >= 0; j--){ // Check if duplicate - // TMP TMP - merge with device search improvements + // TODO(themarpe) - merge with device search improvements // to have mxid available as well if(strcmp(devices[i].name, devices[j].name) == 0){ duplicate = true; diff --git a/src/shared/XLinkDevice.c b/src/shared/XLinkDevice.c index b885f037..4fe0a1e5 100644 --- a/src/shared/XLinkDevice.c +++ b/src/shared/XLinkDevice.c @@ -224,7 +224,9 @@ XLinkError_t XLinkConnectWithTimeout(XLinkHandler_t* handler, const unsigned int DispatcherAddEvent(EVENT_LOCAL, &event); if (DispatcherWaitEventComplete(&link->deviceHandle, msTimeout)) { - DispatcherDeviceFdDown(&link->deviceHandle); + DispatcherClean(&link->deviceHandle); + // TODO(themarpe) - cleaner exit in case of timeout... + // DispatcherDeviceFdDown(&link->deviceHandle); return X_LINK_TIMEOUT; } @@ -292,32 +294,37 @@ XLinkError_t XLinkResetRemote(const linkId_t id) } XLinkError_t XLinkResetRemoteTimeout(const linkId_t id, const unsigned int msTimeout) { - xLinkDeviceHandle_t deviceHandle; - XLINK_RET_IF(getLinkUpDeviceHandleByLinkId(id, &deviceHandle)); + // TODO(themarpe) + // xLinkDeviceHandle_t deviceHandle; + // XLINK_RET_IF(getLinkUpDeviceHandleByLinkId(id, &deviceHandle)); + xLinkDesc_t* link = getLinkById(id); + XLINK_RET_IF(link == NULL); // Add event to reset device. After sending it, dispatcher will close fd link xLinkEvent_t event = {0}; event.header.type = XLINK_RESET_REQ; - event.deviceHandle = deviceHandle; + // event.deviceHandle = deviceHandle; + event.deviceHandle = link->deviceHandle; mvLog(MVLOG_DEBUG, "sending reset remote event\n"); DispatcherAddEvent(EVENT_LOCAL, &event); - XLinkError_t ret = DispatcherWaitEventComplete(&deviceHandle, msTimeout); + // XLinkError_t ret = DispatcherWaitEventComplete(&deviceHandle, msTimeout); + XLinkError_t ret = DispatcherWaitEventComplete(&link->deviceHandle, msTimeout); if(ret != X_LINK_SUCCESS){ // Closing device link unblocks any blocked events // Afterwards the dispatcher can properly cleanup in its own thread - DispatcherDeviceFdDown(&deviceHandle); + // DispatcherDeviceFdDown(&deviceHandle); + DispatcherDeviceFdDown(&link->deviceHandle); } - // TMP TMP - investigate if not waiting for the dispatcher to shutdown is ok - // int rc; - // while(((rc = XLink_sem_wait(&link->dispatcherClosedSem)) == -1) && errno == EINTR) - // continue; - // if(rc) { - // mvLog(MVLOG_ERROR,"can't wait dispatcherClosedSem\n"); - // return X_LINK_ERROR; - // } + int rc; + while(((rc = XLink_sem_wait(&link->dispatcherClosedSem)) == -1) && errno == EINTR) + continue; + if(rc) { + mvLog(MVLOG_ERROR,"can't wait dispatcherClosedSem\n"); + return X_LINK_ERROR; + } return ret; } diff --git a/src/shared/XLinkDispatcher.c b/src/shared/XLinkDispatcher.c index 8ec0b11e..80299eca 100644 --- a/src/shared/XLinkDispatcher.c +++ b/src/shared/XLinkDispatcher.c @@ -447,8 +447,12 @@ int DispatcherWaitEventComplete(xLinkDeviceHandle_t *deviceHandle, unsigned int while(((rc = XLink_sem_wait(id)) == -1) && errno == EINTR) continue; if (id == NULL || rc) { - // graceful link shutdown instead - dispatcherDeviceFdDown(curr); + // // graceful link shutdown instead + // dispatcherDeviceFdDown(curr); + + // Calling non-thread safe dispatcherReset from external thread + // TODO - investigate further and resolve + dispatcherReset(curr); } } #endif diff --git a/src/shared/XLinkDispatcherImpl.c b/src/shared/XLinkDispatcherImpl.c index 50ca7812..13f9955b 100644 --- a/src/shared/XLinkDispatcherImpl.c +++ b/src/shared/XLinkDispatcherImpl.c @@ -88,7 +88,7 @@ int dispatcherEventReceive(xLinkEvent_t* event){ return rc; } - // TODO - reimplement duplicate ID detection + // TODO(themarpe) - reimplement duplicate ID detection // if (prevEvent.header.id == event->header.id && // prevEvent.header.type == event->header.type && // prevEvent.deviceHandle.xLinkFD == event->deviceHandle.xLinkFD) { From acdf27b89250a8560c5c91f9ac2ea3c58aad2950 Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Tue, 8 Mar 2022 02:37:24 +0100 Subject: [PATCH 07/14] Added unique keys for usb device handles --- src/pc/PlatformData.c | 24 ++++++++++++++++++++---- src/pc/PlatformDeviceControl.c | 23 +++++++++++++++-------- src/shared/XLinkDevice.c | 1 + 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/pc/PlatformData.c b/src/pc/PlatformData.c index 3fb852ce..80e78a4a 100644 --- a/src/pc/PlatformData.c +++ b/src/pc/PlatformData.c @@ -169,7 +169,7 @@ void XLinkPlatformDeallocateData(void *ptr, uint32_t size, uint32_t alignment) // Wrappers implementation. Begin. // ------------------------------------ -int usbPlatformRead(void* fd, void* data, int size) +int usbPlatformRead(void* fdKey, void* data, int size) { int rc = 0; #ifndef USE_USB_VSC @@ -209,12 +209,20 @@ int usbPlatformRead(void* fd, void* data, int size) } #endif /*USE_LINK_JTAG*/ #else - rc = usb_read((libusb_device_handle *) fd, data, size); + + void* tmpUsbHandle = NULL; + if(getPlatformDeviceFdFromKey(fdKey, &tmpUsbHandle)){ + mvLog(MVLOG_FATAL, "Cannot find file descriptor by key: %" PRIxPTR, (uintptr_t) fdKey); + return -1; + } + libusb_device_handle* usbHandle = (libusb_device_handle*) tmpUsbHandle; + + rc = usb_read(usbHandle, data, size); #endif /*USE_USB_VSC*/ return rc; } -int usbPlatformWrite(void *fd, void *data, int size) +int usbPlatformWrite(void *fdKey, void *data, int size) { int rc = 0; #ifndef USE_USB_VSC @@ -257,7 +265,15 @@ int usbPlatformWrite(void *fd, void *data, int size) } #endif /*USE_LINK_JTAG*/ #else - rc = usb_write((libusb_device_handle *) fd, data, size); + + void* tmpUsbHandle = NULL; + if(getPlatformDeviceFdFromKey(fdKey, &tmpUsbHandle)){ + mvLog(MVLOG_FATAL, "Cannot find file descriptor by key: %" PRIxPTR, (uintptr_t) fdKey); + return -1; + } + libusb_device_handle* usbHandle = (libusb_device_handle*) tmpUsbHandle; + + rc = usb_write(usbHandle, data, size); #endif /*USE_USB_VSC*/ return rc; } diff --git a/src/pc/PlatformDeviceControl.c b/src/pc/PlatformDeviceControl.c index a8c46a6c..152acac8 100644 --- a/src/pc/PlatformDeviceControl.c +++ b/src/pc/PlatformDeviceControl.c @@ -617,17 +617,18 @@ int usbPlatformConnect(const char *devPathRead, const char *devPathWrite, void * return 0; #endif /*USE_LINK_JTAG*/ #else - *fd = usbLinkOpen(devPathWrite); - if (*fd == 0) + void* usbHandle = usbLinkOpen(devPathWrite); + + if (usbHandle == 0) { /* could fail due to port name change */ return -1; } - if(*fd) - return 0; - else - return -1; + // Store the usb handle and create a "unique" key instead + // (as file descriptors are reused and can cause a clash with lookups between scheduler and link) + *fd = createPlatformDeviceFdKey(usbHandle); + #endif /*USE_USB_VSC*/ } @@ -724,7 +725,7 @@ int tcpipPlatformBootBootloader(const char *name) return tcpip_boot_bootloader(name); } -int usbPlatformClose(void *fd) +int usbPlatformClose(void *fdKey) { #ifndef USE_USB_VSC @@ -741,7 +742,13 @@ int usbPlatformClose(void *fd) } #endif /*USE_LINK_JTAG*/ #else - usbLinkClose((libusb_device_handle *) fd); + + void* tmpUsbHandle = NULL; + if(getPlatformDeviceFdFromKey(fdKey, &tmpUsbHandle)){ + mvLog(MVLOG_FATAL, "Cannot find USB Handle by key"); + return -1; + } + usbLinkClose((libusb_device_handle *) tmpUsbHandle); #endif /*USE_USB_VSC*/ return -1; } diff --git a/src/shared/XLinkDevice.c b/src/shared/XLinkDevice.c index 4fe0a1e5..924498ed 100644 --- a/src/shared/XLinkDevice.c +++ b/src/shared/XLinkDevice.c @@ -179,6 +179,7 @@ XLinkError_t XLinkFindAllSuitableDevices(XLinkDeviceState_t state, } //Called only from app - per device +XLinkError_t XLinkConnectWithTimeout(XLinkHandler_t* handler, const unsigned int msTimeout); XLinkError_t XLinkConnect(XLinkHandler_t* handler) { return XLinkConnectWithTimeout(handler, XLINK_NO_RW_TIMEOUT); From a1a9e558ea9a5ac7f327bf68b0e6338aac194bce Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Tue, 8 Mar 2022 03:44:15 +0100 Subject: [PATCH 08/14] WIP: Fixed DispatcherWaitEventComplete --- src/pc/PlatformDeviceControl.c | 8 +++++++- src/shared/XLinkDispatcher.c | 7 ++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/pc/PlatformDeviceControl.c b/src/pc/PlatformDeviceControl.c index 152acac8..9b0e3b1e 100644 --- a/src/pc/PlatformDeviceControl.c +++ b/src/pc/PlatformDeviceControl.c @@ -749,6 +749,12 @@ int usbPlatformClose(void *fdKey) return -1; } usbLinkClose((libusb_device_handle *) tmpUsbHandle); + + if(destroyPlatformDeviceFdKey(fdKey)){ + mvLog(MVLOG_FATAL, "Cannot destroy USB Handle key"); + return -1; + } + #endif /*USE_USB_VSC*/ return -1; } @@ -801,7 +807,7 @@ int tcpipPlatformClose(void *fdKey) #endif if(destroyPlatformDeviceFdKey(fdKey)){ - mvLog(MVLOG_FATAL, "Cannot destory file descriptor key"); + mvLog(MVLOG_FATAL, "Cannot destroy file descriptor key"); return -1; } diff --git a/src/shared/XLinkDispatcher.c b/src/shared/XLinkDispatcher.c index 80299eca..26049131 100644 --- a/src/shared/XLinkDispatcher.c +++ b/src/shared/XLinkDispatcher.c @@ -421,14 +421,15 @@ int DispatcherWaitEventComplete(xLinkDeviceHandle_t *deviceHandle, unsigned int // This is a temporary solution. TODO: replace this with something more efficient. while (timeoutMs--) { rc = XLink_sem_trywait(id); - if (!rc) { - break; - } else { + printf("rc = %d\n", rc); + if (rc == ETIMEDOUT) { #if (defined(_WIN32) || defined(_WIN64) ) Sleep(1); #else usleep(1000); #endif + } else { + break; } } } else { From 41a665f7c324a98776704fcee6ab696110edcb58 Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Tue, 8 Mar 2022 04:01:00 +0100 Subject: [PATCH 09/14] Added flags and fixed a return value --- CMakeLists.txt | 1 + cmake/Flags.cmake | 6 +++--- src/pc/PlatformDeviceControl.c | 2 ++ src/pc/protocols/tcpip_host.c | 8 ++++---- src/shared/XLinkDispatcher.c | 1 - 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0650dc67..78709498 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,6 +16,7 @@ option(XLINK_BUILD_EXAMPLES "Build XLink examples" OFF) option(XLINK_BUILD_TESTS "Build XLink tests" OFF) add_library(${TARGET_NAME} ${XLINK_SOURCES}) +add_default_flags(${TARGET_NAME}) add_flag_source(src/shared/XLinkDevice.c "-Werror=switch-enum") if(WIN32) diff --git a/cmake/Flags.cmake b/cmake/Flags.cmake index 23b4b527..08c1e2fc 100644 --- a/cmake/Flags.cmake +++ b/cmake/Flags.cmake @@ -18,7 +18,7 @@ endfunction() function(add_default_flags target) if ("${CMAKE_C_COMPILER_ID}" MATCHES "^(AppleClang|Clang|GNU)$") # enable those flags - add_flag(${target} -Woverloaded-virtual) # warn if you overload (not override) a virtual function + # add_flag(${target} -Woverloaded-virtual) # warn if you overload (not override) a virtual function add_flag(${target} -Wformat=2) # warn on security issues around functions that format output (ie printf) add_flag(${target} -Wmisleading-indentation) # (only in GCC >= 6.0) warn if indentation implies blocks where blocks do not exist add_flag(${target} -Wduplicated-cond) # (only in GCC >= 6.0) warn if if / else chain has duplicated conditions @@ -37,9 +37,9 @@ function(add_default_flags target) add_flag(${target} -Werror=self-assign-field) # error if self assign - bugprone add_flag(${target} -Werror=unused-lambda-capture) # error if lambda capture is unused add_flag(${target} -Werror=return-type) # warning: control reaches end of non-void function [-Wreturn-type] - add_flag(${target} -Werror=non-virtual-dtor) # warn the user if a class with virtual functions has a non-virtual destructor. This helps catch hard to track down memory errors + # add_flag(${target} -Werror=non-virtual-dtor) # warn the user if a class with virtual functions has a non-virtual destructor. This helps catch hard to track down memory errors add_flag(${target} -Werror=sign-compare) # warn the user if they compare a signed and unsigned numbers - add_flag(${target} -Werror=reorder) # field '$1' will be initialized after field '$2' + # add_flag(${target} -Werror=reorder) # field '$1' will be initialized after field '$2' elseif ("${CMAKE_C_COMPILER_ID}" STREQUAL "MSVC") # using Visual Studio C++ diff --git a/src/pc/PlatformDeviceControl.c b/src/pc/PlatformDeviceControl.c index 9b0e3b1e..2e00b901 100644 --- a/src/pc/PlatformDeviceControl.c +++ b/src/pc/PlatformDeviceControl.c @@ -630,6 +630,8 @@ int usbPlatformConnect(const char *devPathRead, const char *devPathWrite, void * *fd = createPlatformDeviceFdKey(usbHandle); #endif /*USE_USB_VSC*/ + + return 0; } int pciePlatformConnect(UNUSED const char *devPathRead, diff --git a/src/pc/protocols/tcpip_host.c b/src/pc/protocols/tcpip_host.c index 6ce966df..23886a22 100644 --- a/src/pc/protocols/tcpip_host.c +++ b/src/pc/protocols/tcpip_host.c @@ -309,7 +309,7 @@ xLinkPlatformErrorCode_t tcpip_get_devices(XLinkDeviceState_t state, deviceDesc_ } // loop to receive message response from devices - int num_devices_match = 0; + size_t num_devices_match = 0; // Loop through all sockets and received messages that arrived double t1 = seconds(); do { @@ -357,10 +357,10 @@ xLinkPlatformErrorCode_t tcpip_get_devices(XLinkDeviceState_t state, deviceDesc_ // Filter out duplicates - routing table will decide through which interface the packets will traverse // TODO(themarpe) - properly separate interfaces. // Either bind to interface addr, or SO_BINDTODEVICE Linux, IP_BOUND_IF macOS, and prefix interface name - int write_index = 0; - for(int i = 0; i < (int) num_devices_match; i++){ + ssize_t write_index = 0; + for(ssize_t i = 0; i < (ssize_t) num_devices_match; i++){ bool duplicate = false; - for(int j = i - 1; j >= 0; j--){ + for(ssize_t j = i - 1; j >= 0; j--){ // Check if duplicate // TODO(themarpe) - merge with device search improvements diff --git a/src/shared/XLinkDispatcher.c b/src/shared/XLinkDispatcher.c index 26049131..1468202a 100644 --- a/src/shared/XLinkDispatcher.c +++ b/src/shared/XLinkDispatcher.c @@ -421,7 +421,6 @@ int DispatcherWaitEventComplete(xLinkDeviceHandle_t *deviceHandle, unsigned int // This is a temporary solution. TODO: replace this with something more efficient. while (timeoutMs--) { rc = XLink_sem_trywait(id); - printf("rc = %d\n", rc); if (rc == ETIMEDOUT) { #if (defined(_WIN32) || defined(_WIN64) ) Sleep(1); From 3db5a9a1f44b51be6c10f824578e6a6956579c52 Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Tue, 8 Mar 2022 04:19:34 +0100 Subject: [PATCH 10/14] WIP: Fixed timeout wait --- src/pc/Win/src/win_semaphore.c | 9 +++++++-- src/shared/XLinkDispatcher.c | 17 ++++++++++++----- src/shared/XLinkSemaphore.c | 2 ++ 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/pc/Win/src/win_semaphore.c b/src/pc/Win/src/win_semaphore.c index dc9ad6fb..34bc572e 100644 --- a/src/pc/Win/src/win_semaphore.c +++ b/src/pc/Win/src/win_semaphore.c @@ -94,10 +94,15 @@ int sem_trywait(sem_t *sem){ return ls_set_errno(EINVAL); } sem_t s = *sem; - if (WaitForSingleObject(s->handle, 0) != WAIT_OBJECT_0) { + + DWORD ret = WaitForSingleObject(s->handle, 0); + if (ret == WAIT_OBJECT_0) { + return 0; + } else if(ret == WAIT_TIMEOUT) { + return ls_set_errno(ETIMEDOUT); + } else { return ls_set_errno(EINVAL); } - return 0; } diff --git a/src/shared/XLinkDispatcher.c b/src/shared/XLinkDispatcher.c index 1468202a..804270a2 100644 --- a/src/shared/XLinkDispatcher.c +++ b/src/shared/XLinkDispatcher.c @@ -421,14 +421,21 @@ int DispatcherWaitEventComplete(xLinkDeviceHandle_t *deviceHandle, unsigned int // This is a temporary solution. TODO: replace this with something more efficient. while (timeoutMs--) { rc = XLink_sem_trywait(id); - if (rc == ETIMEDOUT) { + int tmpErrno = errno; + if (rc == 0) { + // Success + break; + } else { + if(tmpErrno == ETIMEDOUT) { #if (defined(_WIN32) || defined(_WIN64) ) - Sleep(1); + Sleep(1); #else - usleep(1000); + usleep(1000); #endif - } else { - break; + } else { + // error, exit + break; + } } } } else { diff --git a/src/shared/XLinkSemaphore.c b/src/shared/XLinkSemaphore.c index 40d90271..1f2b610e 100644 --- a/src/shared/XLinkSemaphore.c +++ b/src/shared/XLinkSemaphore.c @@ -121,8 +121,10 @@ int XLink_sem_trywait(XLink_sem_t* sem) XLINK_RET_IF_FAIL(XLink_sem_inc(sem)); int ret = sem_trywait(&sem->psem); + int tmpErrno = errno; XLINK_RET_IF_FAIL(XLink_sem_dec(sem)); + errno = tmpErrno; return ret; } From 3192807e019abce53751118dcb6a920980b3e1f4 Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Tue, 8 Mar 2022 04:50:57 +0100 Subject: [PATCH 11/14] Modified back to standard type --- src/pc/protocols/tcpip_host.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pc/protocols/tcpip_host.c b/src/pc/protocols/tcpip_host.c index 23886a22..0098ba9d 100644 --- a/src/pc/protocols/tcpip_host.c +++ b/src/pc/protocols/tcpip_host.c @@ -357,10 +357,10 @@ xLinkPlatformErrorCode_t tcpip_get_devices(XLinkDeviceState_t state, deviceDesc_ // Filter out duplicates - routing table will decide through which interface the packets will traverse // TODO(themarpe) - properly separate interfaces. // Either bind to interface addr, or SO_BINDTODEVICE Linux, IP_BOUND_IF macOS, and prefix interface name - ssize_t write_index = 0; - for(ssize_t i = 0; i < (ssize_t) num_devices_match; i++){ + int write_index = 0; + for(int i = 0; i < (int) num_devices_match; i++){ bool duplicate = false; - for(ssize_t j = i - 1; j >= 0; j--){ + for(int j = i - 1; j >= 0; j--){ // Check if duplicate // TODO(themarpe) - merge with device search improvements From cd4708f71d74de48e45f524c02d8b43978264019 Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Tue, 8 Mar 2022 16:33:22 +0100 Subject: [PATCH 12/14] Windows fix race when searching for devices --- src/pc/PlatformData.c | 2 +- src/pc/Win/src/win_usb.c | 36 +++++++++++++++++++++++++++++++----- src/shared/XLinkSemaphore.c | 1 + 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/pc/PlatformData.c b/src/pc/PlatformData.c index 80e78a4a..76bc4885 100644 --- a/src/pc/PlatformData.c +++ b/src/pc/PlatformData.c @@ -293,7 +293,7 @@ int pciePlatformWrite(void *f, void *data, int size) { write_pending = 1; - size_t chunk = size < CHUNK_SIZE_BYTES ? size : CHUNK_SIZE_BYTES; + size_t chunk = (size_t)size < CHUNK_SIZE_BYTES ? (size_t)size : CHUNK_SIZE_BYTES; int num_written = pcie_write(f, data, chunk); write_pending = 0; diff --git a/src/pc/Win/src/win_usb.c b/src/pc/Win/src/win_usb.c index 10a9a636..f06d6e33 100644 --- a/src/pc/Win/src/win_usb.c +++ b/src/pc/Win/src/win_usb.c @@ -19,6 +19,7 @@ #include "usb_boot.h" #include "usb_mx_id.h" #include "stdbool.h" +#include "win_pthread.h" #define MVLOG_UNIT_NAME xLinkWinUsb #include "XLinkLog.h" @@ -76,6 +77,7 @@ static size_t errmsg_buff_len = 0; static int MX_ID_TIMEOUT = 100; // 100ms +static pthread_mutex_t globalMutex = PTHREAD_MUTEX_INITIALIZER; static const char* gen_addr_mx_id(HDEVINFO devInfo, SP_DEVINFO_DATA* devInfoData, int pid, char** refDevicePath); @@ -464,7 +466,7 @@ int usb_bulk_write(usb_hwnd han, uint8_t ep, const void *buffer, size_t sz, uint if(han == NULL) return USB_ERR_INVALID; - if(timeout_ms != han->eps[USB_DIR_OUT].last_timeout) { + if(timeout_ms != (long) han->eps[USB_DIR_OUT].last_timeout) { han->eps[USB_DIR_OUT].last_timeout = timeout_ms; if(!WinUsb_SetPipePolicy(han->winUsbHan, ep, PIPE_TRANSFER_TIMEOUT, sizeof(ULONG), &han->eps[USB_DIR_OUT].last_timeout)) { @@ -494,7 +496,7 @@ int usb_bulk_read(usb_hwnd han, uint8_t ep, void *buffer, size_t sz, uint32_t *r if(han == NULL) return USB_ERR_INVALID; - if(timeout_ms != han->eps[USB_DIR_IN].last_timeout) { + if(timeout_ms != (long) han->eps[USB_DIR_IN].last_timeout) { han->eps[USB_DIR_IN].last_timeout = timeout_ms; if(!WinUsb_SetPipePolicy(han->winUsbHan, ep, PIPE_TRANSFER_TIMEOUT, sizeof(ULONG), &han->eps[USB_DIR_IN].last_timeout)) { @@ -894,7 +896,11 @@ int win_usb_find_device(unsigned idx, char* addr, unsigned addrsize, void** devi if (strlen(addr) > 1) specificDevice = 1; - // TODO There is no global mutex as in linux version + if (pthread_mutex_lock(&globalMutex)) { + mvLog(MVLOG_ERROR, "globalMutex lock failed"); + return USB_BOOT_ERROR; + } + int res; static usb_dev_list* devs = NULL; @@ -909,6 +915,10 @@ int win_usb_find_device(unsigned idx, char* addr, unsigned addrsize, void** devi } if ((res = usb_get_device_list(&devs)) < 0) { mvLog(MVLOG_DEBUG, "Unable to get USB device list: %s", libusb_strerror(res)); + + if (pthread_mutex_unlock(&globalMutex)) { + mvLog(MVLOG_ERROR, "globalMutex unlock failed"); + } return USB_BOOT_ERROR; } devs_cnt = res; @@ -954,6 +964,10 @@ int win_usb_find_device(unsigned idx, char* addr, unsigned addrsize, void** devi // Create a copy of device path string. It should be freed by usb_free_device() *device = _strdup(devicePath); devs_cnt = 0; + + if (pthread_mutex_unlock(&globalMutex)) { + mvLog(MVLOG_ERROR, "globalMutex unlock failed"); + } return USB_BOOT_SUCCESS; } } @@ -962,25 +976,37 @@ int win_usb_find_device(unsigned idx, char* addr, unsigned addrsize, void** devi // gen addr const char* caddr = gen_addr_mx_id(devs->devInfo, devs->infos + i, idProduct, NULL); - if (strncmp(addr, caddr, XLINK_MAX_NAME_SIZE) == 0) + if (addr && caddr && strncmp(addr, caddr, XLINK_MAX_NAME_SIZE) == 0) { mvLog(MVLOG_DEBUG, "Found Address: %s - VID/PID %04x:%04x", caddr, idVendor, idProduct, NULL); + + if (pthread_mutex_unlock(&globalMutex)) { + mvLog(MVLOG_ERROR, "globalMutex unlock failed"); + } return USB_BOOT_SUCCESS; } } - else if (idx == count) + else if ((int) idx == count) { // gen addr const char* caddr = gen_addr_mx_id(devs->devInfo, devs->infos + i, idProduct, NULL); mvLog(MVLOG_DEBUG, "Device %d Address: %s - VID/PID %04x:%04x", idx, caddr, idVendor, idProduct); mv_strncpy(addr, addrsize, caddr, addrsize - 1); + + if (pthread_mutex_unlock(&globalMutex)) { + mvLog(MVLOG_ERROR, "globalMutex unlock failed"); + } return USB_BOOT_SUCCESS; } count++; } } devs_cnt = 0; + + if (pthread_mutex_unlock(&globalMutex)) { + mvLog(MVLOG_ERROR, "globalMutex unlock failed"); + } return USB_BOOT_DEVICE_NOT_FOUND; } #endif diff --git a/src/shared/XLinkSemaphore.c b/src/shared/XLinkSemaphore.c index 1f2b610e..fe8dd65e 100644 --- a/src/shared/XLinkSemaphore.c +++ b/src/shared/XLinkSemaphore.c @@ -117,6 +117,7 @@ int XLink_sem_timedwait(XLink_sem_t* sem, const struct timespec* abstime) int XLink_sem_trywait(XLink_sem_t* sem) { + errno = EINVAL; XLINK_RET_ERR_IF(sem == NULL, -1); XLINK_RET_IF_FAIL(XLink_sem_inc(sem)); From 0bdc3b929f78540619bd1768b7a6ef1346619a69 Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Tue, 8 Mar 2022 16:58:38 +0100 Subject: [PATCH 13/14] Fixed compilation issues --- src/pc/PlatformDeviceControl.c | 15 ++++++++++++--- src/pc/Win/src/win_usb.c | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/pc/PlatformDeviceControl.c b/src/pc/PlatformDeviceControl.c index 2e00b901..d778654b 100644 --- a/src/pc/PlatformDeviceControl.c +++ b/src/pc/PlatformDeviceControl.c @@ -645,14 +645,23 @@ int pciePlatformConnect(UNUSED const char *devPathRead, int tcpipPlatformConnect(const char *devPathRead, const char *devPathWrite, void **fd) { #if defined(USE_TCP_IP) - if (!devPathWrite || !fd) + if (!devPathWrite || !fd) { return X_LINK_PLATFORM_INVALID_PARAMETERS; + } + TCPIP_SOCKET sock = socket(AF_INET, SOCK_STREAM, 0); + +#if (defined(_WIN32) || defined(_WIN64) ) + if(sock == INVALID_SOCKET) + { + return TCPIP_HOST_ERROR; + } +#else if(sock < 0) { - tcpip_close_socket(sock); - return -1; + return TCPIP_HOST_ERROR; } +#endif // Disable sigpipe reception on send #if defined(SO_NOSIGPIPE) diff --git a/src/pc/Win/src/win_usb.c b/src/pc/Win/src/win_usb.c index f06d6e33..b5b069c3 100644 --- a/src/pc/Win/src/win_usb.c +++ b/src/pc/Win/src/win_usb.c @@ -731,7 +731,7 @@ static const char* gen_addr_mx_id(HDEVINFO devInfo, SP_DEVINFO_DATA* devInfoData rbuf[8] &= 0xF0; // Convert to HEX presentation and store into mx_id - for (uint32_t i = 0; i < expectedMxIdReadSize; i++) + for (int i = 0; i < expectedMxIdReadSize; i++) { sprintf(mx_id + (2 * (uintptr_t)i), "%02X", rbuf[i]); } From 51c2a44573bf52ccdd8cc5121d67a62b1dbd331d Mon Sep 17 00:00:00 2001 From: Martin Peterlin Date: Tue, 8 Mar 2022 23:38:13 +0100 Subject: [PATCH 14/14] Some additional Windows changes --- src/pc/protocols/tcpip_host.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/pc/protocols/tcpip_host.c b/src/pc/protocols/tcpip_host.c index 0098ba9d..b1a57270 100644 --- a/src/pc/protocols/tcpip_host.c +++ b/src/pc/protocols/tcpip_host.c @@ -309,11 +309,11 @@ xLinkPlatformErrorCode_t tcpip_get_devices(XLinkDeviceState_t state, deviceDesc_ } // loop to receive message response from devices - size_t num_devices_match = 0; + int num_devices_match = 0; // Loop through all sockets and received messages that arrived double t1 = seconds(); do { - if(num_devices_match >= devices_size){ + if(num_devices_match >= (long) devices_size){ // Enough devices matched, exit the loop break; } @@ -321,8 +321,12 @@ xLinkPlatformErrorCode_t tcpip_get_devices(XLinkDeviceState_t state, deviceDesc_ char ip_addr[INET_ADDRSTRLEN] = {0}; tcpipHostDeviceDiscoveryResp_t recv_buffer = {0}; struct sockaddr_in dev_addr; - uint32_t len = sizeof(dev_addr); - + #if (defined(_WIN32) || defined(_WIN64) ) + int len = sizeof(dev_addr); + #else + socklen_t len = sizeof(dev_addr); + #endif + int ret = recvfrom(sock, (char *) &recv_buffer, sizeof(recv_buffer), 0, (struct sockaddr*) & dev_addr, &len); if(ret > 0) { @@ -358,7 +362,7 @@ xLinkPlatformErrorCode_t tcpip_get_devices(XLinkDeviceState_t state, deviceDesc_ // TODO(themarpe) - properly separate interfaces. // Either bind to interface addr, or SO_BINDTODEVICE Linux, IP_BOUND_IF macOS, and prefix interface name int write_index = 0; - for(int i = 0; i < (int) num_devices_match; i++){ + for(int i = 0; i < num_devices_match; i++){ bool duplicate = false; for(int j = i - 1; j >= 0; j--){ // Check if duplicate