aseba-community / dashel Goto Github PK

What about throwing exceptions instead of calling abort? See dashel/dashel-win32.cpp and dashel/dashel-posix.cpp

Dashel has no unit nor end-to-end tests. At least basic end-to-end tests should be present.

The dashelConfig.cmake CMake configuration file introduced in 68d19e1 returns empty variables $dashel_INCLUDE_DIRS and $dashel_LIBRARY when used like that:

find_package(dashel REQUIRED)

Tested on Ubuntu 14.04 with cmake-2.8.12.

Port enumeration on MacOS no longer works since 8ab28e8 because the IOKit code is not conditionally included. (Observed when compiling with CMake and Xcode.)

According to OS X 10.11's /usr/include/TargetConditionals.h, TARGET_OS_IPHONE is always defined. Its value is 0 or 1 depending on the target. The test on lines 137 and 77 should be:

#if defined MACOSX && TARGET_OS_IPHONE == 0

Note that the two forms are mixed because MACOSX is defined, or not, but TARGET_OS_IPHONE is always defined.

On Windows versions before 10, using Thymio provided USB serial (rfcomm) driver, the disconnection of the physical device leads to a write failure, leading Dashel to be aware of the disconnection.

On Windows 10, which provides its own rfcomm driver, the physical disconnection of the device does not lead to a write failure, so Dashel cannot realise that the disconnection happened.

This is not a bug in Dashel, but in the Windows 10 driver. There might be a work-around by creating a hidden windows that listens to the system event for the USB device itself, but it is super-complicated.

USING: Windows 10 Pro 64-bit OS

On the file 'dashel/dashel/dashel-win32.cpp ', there is a bug on the method 'bool Hub::step(const)' which incurs in not closing appropriately clients connections, and therefore keeps listening to the past clients whose connection should had been destroyed.

This generates either a collapse of the hub method (since a 64 position vector gets filled up), or a timeout on the connection.

Our hypothesis is that there's a function called 'notifyEvent' which changes the vector ets[ ], thus changing the behavior of the conditionals that decide when to connect,receive data and disconnect. We changed the position in which this function is called, an managed to make it "kind" of work. A client can indeed connect and disconnect from the hub but the first time it connects it produces a timeout error. From here on it continues working.

This happens because when Client 1 connects at event = 0 and Client 2 connects at event = 1 and so on, each client should disconnect on the same event in which it made the connection. However Client 1 is disconnecting at event = 1 and Client 2 at event = 2; so i think this is the mistake

$ make
d:/msys/home/michel/develop/webots-master/projects/robots/thymio/libraries/dashel-src/dashel/dashel-win32.cpp:58:0: warning: "_WIN32_WINNT" redefined [enabled by default]
 #define _WIN32_WINNT 0x0501
 ^
In file included from d:/msys/home/michel/develop/webots-master/mingw/i686-w64-mingw32/include/crtdefs.h:10:0,
                 from d:/msys/home/michel/develop/webots-master/mingw/i686-w64-mingw32/include/wchar.h:9,
                 from d:/msys/home/michel/develop/webots-master/mingw/i686-w64-mingw32/include/c++/cwchar:44,
                 from d:/msys/home/michel/develop/webots-master/mingw/i686-w64-mingw32/include/c++/bits/postypes.h:40,
                 from d:/msys/home/michel/develop/webots-master/mingw/i686-w64-mingw32/include/c++/bits/char_traits.h:40,
                 from d:/msys/home/michel/develop/webots-master/mingw/i686-w64-mingw32/include/c++/string:40,
                 from d:/msys/home/michel/develop/webots-master/projects/robots/thymio/libraries/dashel-src/dashel/dashel.h:44,
                 from d:/msys/home/michel/develop/webots-master/projects/robots/thymio/libraries/dashel-src/dashel/dashel-win32.cpp:41:
d:/msys/home/michel/develop/webots-master/mingw/i686-w64-mingw32/include/_mingw.h:229:0: note: this is the location of the previous definition
 #define _WIN32_WINNT 0x502
 ^

Can be fixed by the following patch:

diff --git a/dashel/dashel-win32.cpp b/dashel/dashel-win32.cpp
index b55c2a4..c0df90a 100644
--- a/dashel/dashel-win32.cpp
+++ b/dashel/dashel-win32.cpp
@@ -55,6 +55,9 @@
        #pragma comment(lib, "comsuppw.lib")
 #endif // _MSC_VER

+#ifdef _WIN32_WINNT
+#undef _WIN32_WINNT
+#endif
 #define _WIN32_WINNT 0x0501
 #include <winsock2.h>
 #include <windows.h>

However, I am unable to create a branch on this repository to submit this patch for review. Can you help me?

Currently, on Windows, target stdin: does not work on msys2/cygwin command line windows. An exception is generated by the call SetConsoleMode(...) at line 407 of dashel-win32.cpp. The cause is similar to the one in this post. It would be nice to have stdin: working with these windows as well.

File dashel/dashel.h states that either port or device must be given. It should be either port, name (on supported platforms), or device must be given.

I'm hitting a bug in asebahttp, where disconnected the device connected over usb ( serial interface ) crash the application on linux.
This boils down to the kernel sending SIGHUP within poll.
CLOCAL prevents that signal to be emitted on some posix implementations (bsd, OSX), but not all (linux)
A solution would be to handle SIGHUP properly.

In dashel-posix.cpp and dashel-win32.cpp, constructors of Stream subclasses call explicitly the Stream constructor even when there is an intermediate class in the class hierarchy and they already call the Stream constructor indirectly. For example in dashel-posix.cpp, SocketStream::SocketStream calls both Stream("tcp") and DisconnectableStream("tcp") while DisconnectableStream::DisconnectableStream already calls Stream::Stream with the correct parameter.