wjasper / Linux_Drivers

Open source Linux device drivers
GNU General Public License v3.0
110 stars 64 forks source link

The driver for network device E-DIO24 is not stable #11

Closed yhfudev closed 6 years ago

yhfudev commented 6 years ago

The driver may fail if there's delay in a network. It is the problem of the functions which handle TCP connection in the E-DIO24 library. A more reliable method to handle TCP connection is to use event driven paradigm. I would suggest separating the packet generating and network routines.

For these reasons, I created a stand alone project for the E-DIO24 devices to show the idea. The project includes a packet generating library, a edio24cli program to control the device by command line, and a simulator edio24sim for debug. More details is at the project github: https://github.com/yhfudev/libedio24.git

wjasper commented 6 years ago

I'm not sure what you mean. First, I disagree that the driver I wrote is not stable. Second, I separated the packet generating part from the network routines. The network routines are in a common file ethernet.c. which listens on a socket via a select statement. Have you measured the amount of delay on your network? Try increasing the timeout in select(). TCP by design is not hard real-time. If you need hard real-time, try a deterministic bus like PCI.

yhfudev commented 6 years ago

I mean the timeout of the function receiveMessage() is set to 1000 ms. I have a device connected with WiFi, and packet loss or some other issues may cause the packet delayed larger than 1 second. This cause the function receiveMessage() failed. You may analysis what problem will occurred if it failed.

While your implementation of the E-DIO24 driver integrate both the packet and network routine together, it is not convenient for the user to integrate to their applications. Just as suggested by this https://github.com/wjasper/Linux_Drivers/issues/5, keep the library as small as possible, do not invoke too many functions in a library for other developers, such as exit(), or receiveMessage() in E-DIO24.

In the example project (https://github.com/yhfudev/libedio24.git), the routines to create protocol packets were in the library, and network related functions, such as create socket, send/recv, control flows etc, were given as examples (not a part of library) provided in the directory utils. The developers can choose just the library and implemented theirs network routines, or choose to use the executable in the directory utils to test their devices. The examples in the directory utils are implemented with the event driven library libuv.

wjasper commented 6 years ago

If you have severe packet delay or packet loss or an unreliable communications channel, you are going to have problems no matter how you implement things for data acquisition. Are you comments "hypothetical" or based on actual experience? Be careful in criticizing an implementation without providing extensive testing. I disagree with you in having the user provide all the network routines in their own code at the socket level. If you want to promote your own code, don't do so my repo. If you want to fork the repo and make a useful contribution, please do so. The comments from #5 were towards the USB implementation.

yhfudev commented 6 years ago

I used a devices with 16 E-DIO24 in it. And I invested time to debug to find out the problem when using your Linux driver which I got the link from the mccdaq.com.

You are very proud of your code, while you didn't even test it and fix the problem.

I don't want to criticize how good your code are, I need to integrate the driver to our application and run in a WiFi network. The purpose I show my project as an example is, I don't want to maintain this project, I need get updated when there's upgrade or bug fix. It would better if my changes can merged with yours. But I found it would not possible for the different requirements of my project and your implementation. So that's why I mentioned my project is a example how I use the driver.

My last question is: are you the owner of the mccdaq?