Closed SeanMollet closed 5 years ago
Sean, Great work. Looks very interesting even though I couldn't code Python to save myself. I'm very interested in merging this work but it at least has to work like the original. Just tried the command line usbrelay and it did not output anything. A quick glance at the code shows it is not designed to print anything.
The library also has the state disabled in an #ifdef DEBUG
fprintf(stderr, "Device Found\n type: %04hx %04hx\n path: %s\n
serial_number: %ls", cur_dev->vendor_id, cur_dev->product_id, cur_dev->path, cur_dev->serial_number); fprintf(stderr, "\n"); fprintf(stderr, " Manufacturer: %ls\n", cur_dev->manufacturer_string); fprintf(stderr, " Product: %ls\n", cur_dev->product_string); fprintf(stderr, " Release: %hx\n", cur_dev->release_number); fprintf(stderr, " Interface: %d\n", cur_dev->interface_number);
// The product string is USBRelayx where x is number of relays read
to the \0 in case there are more than 9 fprintf(stderr, " Number of Relays = %d\n", relay_boards[i].relay_count);
//Print the individual relay status
for (int j = 0; j < relay_boards[i].relay_count; j++)
{
if (relay_boards[i].state & 1 << j)
{
printf("%s_%d=1\n", relay_boards[i].serial, j + 1);
}
else
{
printf("%s_%d=0\n", relay_boards[i].serial, j + 1);
}
}
While I am keen to optionally disable debug messages they should be enabled with a command line flag.
Before merging, the usbrelay utility (which everyone uses at this point) should work the same as the old version. It's fine to leave the old debugging output for the moment.
Thank you again for your fine work. Happy to assist. Regards Darryl
On Fri, Mar 1, 2019 at 5:41 AM SeanMollet notifications@github.com wrote:
I realize this is a pretty big pull request, but there really isn't anything out there that is convenient for accessing these boards from python. Yours was the cleanest code to start from, so I used it as a base, moved the guys to a library and implemented a python extension on top of that.
You can view, comment on, or merge this pull request online at:
https://github.com/darrylb123/usbrelay/pull/27 Commit Summary
- Split this into functions, so it can be converted to a library and a command line app
- Converted to a shared library
- Works from python!
- Turned off debugging options
File Changes
- A .gitignore https://github.com/darrylb123/usbrelay/pull/27/files#diff-0 (4)
- M Makefile https://github.com/darrylb123/usbrelay/pull/27/files#diff-1 (25)
- A libusbrelay.c https://github.com/darrylb123/usbrelay/pull/27/files#diff-2 (268)
- R libusbrelay.h https://github.com/darrylb123/usbrelay/pull/27/files#diff-3 (40)
- A libusbrelay_py.c https://github.com/darrylb123/usbrelay/pull/27/files#diff-4 (198)
- A setup.py https://github.com/darrylb123/usbrelay/pull/27/files#diff-5 (13)
- A test.py https://github.com/darrylb123/usbrelay/pull/27/files#diff-6 (25)
- M usbrelay.c https://github.com/darrylb123/usbrelay/pull/27/files#diff-7 (264)
Patch Links:
- https://github.com/darrylb123/usbrelay/pull/27.patch
- https://github.com/darrylb123/usbrelay/pull/27.diff
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/27, or mute the thread https://github.com/notifications/unsubscribe-auth/AGcqVeo-tNoTCw1qCYztYUVa9c45zdhSks5vSDDUgaJpZM4bXlr0 .
Darryl: I've made the changes you requested, the command line is back to working the same way that it did before.
Thanks,
Sean
Sean, Nice job. Works a treat. One niggle, I use Fedora x86_64 so libs need to be installed in /usr/lib64. The Makefile (and setup.py) specify /usr/lib. Not sure of the 'right' way of handling this. Perhaps you do.
I googled and found several suggestions. We could pick one.
Darryl
Just tried building and testing on my raspberry pi that has a 8 way relay board attached and I get these errors when building. $ make cc -shared -fPIC -O2 -Wall -lhidapi-hidraw -o libusbrelay.so libusbrelay.c libusbrelay.c: In function ‘enumerate_relay_boards’: libusbrelay.c:78:4: error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode for (int i = 0; i < relay_board_count; i++) ^ libusbrelay.c:78:4: note: use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code libusbrelay.c:116:13: error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode for (int k = 0; k < relay_boards[i].relay_count; k++) ^ libusbrelay.c: In function ‘find_board’: libusbrelay.c:235:4: error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode for (int i = 0; i < relay_board_count; i++) ^ libusbrelay.c: In function ‘shutdown’: libusbrelay.c:270:4: error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode for (int i = 0; i < relay_board_count; i++) ^ Makefile:9: recipe for target 'libusbrelay.so' failed make: *** [libusbrelay.so] Error 1
the output from cc -v Using built-in specs. COLLECT_GCC=cc COLLECT_LTO_WRAPPER=/usr/lib/gcc/arm-linux-gnueabihf/4.9/lto-wrapper Target: arm-linux-gnueabihf Configured with: ../src/configure -v --with-pkgversion='Raspbian 4.9.2-10' --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.9 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --disable-libitm --disable-libquadmath --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-armhf/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-armhf --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-armhf --with-arch-directory=arm --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --disable-sjlj-exceptions --with-arch=armv6 --with-fpu=vfp --with-float=hard --enable-checking=release --build=arm-linux-gnueabihf --host=arm-linux-gnueabihf --target=arm-linux-gnueabihf Thread model: posix gcc version 4.9.2 (Raspbian 4.9.2-10)
Note that the original code builds and works fine on the same pi. Something to do with the extra -fPIC -shared options. I don't see these errors with your new code on Fedora with gcc 8.2.1
OK, I added a Makefile check for x86_64, which seemed like the sanest choice for /usr/lib64. I also just removed the declarations inside the for loops. That version of GCC apparently will let you have strsep, or for loop declarations, but not both without more flags than I like to pass. So, my for loop declarations went.
I tried building this on my PI and noticed an error since my PI never had the library before. I fixed that, I also added /usr/lib64 into Python's search path for the shared lib. setup.py installs the _py.so library to wherever the distribution it's running on has specified, so that part is automatically sane and portable.
Sean, Small niggle, the makefile still installs in /usr/lib Need to change install -m 0755 libusbrelay.so $(DESTDIR)$(LIBDIR)
Did you test it on the raspberry pi?
Mine does this???
$ ./usbrelay
Error in `./usbrelay': malloc(): memory corruption: 0x01475a10 Aborted
Not sure about your logic in libusbrelay WRT relay boards and why it shows up on the pi and not x86_64. Page size perhaps???
One thing that might be the problem, I originally malloced one extra struct so I could use relay number as the index so from 1-8 rather than 0=7. I used this "feature" to be able to change the relay board id by using index 0 for that.
Tested 2 way and 8 way relay boards, both have malloc errors Darryl
On Sun, Mar 3, 2019 at 9:25 PM SeanMollet notifications@github.com wrote:
I tried building this on my PI and noticed an error since my PI never had the library before. I fixed that, I also added /usr/lib64 into Python's search path for the shared lib. setup.py installs the _py.so library to wherever the distribution it's running on has specified, so that part is automatically sane and portable.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/27#issuecomment-469013445, or mute the thread https://github.com/notifications/unsubscribe-auth/AGcqVVaAyGfFiH0AQXgfSePJZjF-doPzks5vS7E2gaJpZM4bXlr0 .
Lines 4-11 of the makefile: MACHINE := $(shell uname -m)
LIBDIR = /usr/lib
ifeq ($(MACHINE), x86_64) LIBDIR = /usr/lib64 endif
These set LIBDIR if on an x86_64 platform and the later install then stores in /usr/lib64. Is your PI 64 bit? If so, what's the output of uname -m, I'll add that.
On the malloc() error, that's happening on what platform exactly? It happens with relay boards connected?
Libdir is set correctly, but the actual install cp is hard coded to /usr/lib and does not use libdir variable.
The malloc error occurred on a raspberry pi with an 8 way relay attached. I also tested it with a 2 way, same deal. I didn't try it without a relay attached. I'm not very familiar with using gdb so I didn't do more than compiling with -g and running it under gdb. Didn't help me. The pi is running raspian woody.
On Mon., 4 Mar. 2019, 11:03 pm SeanMollet, notifications@github.com wrote:
Lines 4-11 of the makefile: MACHINE := $(shell uname -m)
Default 32 bit x86, raspberry pi, etc..
LIBDIR = /usr/lib
Catch x86_64 machines that use /usr/lib64
ifeq ($(MACHINE), x86_64) LIBDIR = /usr/lib64 endif
These set LIBDIR if on an x86_64 platform and the later install then stores in /usr/lib64. Is your PI 64 bit? If so, what's the output of uname -m, I'll add that.
On the malloc() error, that's happening on what platform exactly? It happens with relay boards connected?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/27#issuecomment-469245372, or mute the thread https://github.com/notifications/unsubscribe-auth/AGcqVQe_r_QUbRAvWcAIt_TNI1erYXq4ks5vTRmogaJpZM4bXlr0 .
Thanks! I finally saw what you were saying, I'd forgotten to change the install to use LIBDIR. I also found and fixed the malloc() bug and added a few more lib paths that debian uses on various platforms.
Ok, that looks good for the compatibility with the old code. As I said before, I know nothing about Python but I tried to use your test program. First problem I ran into was Python-devel puts the includes into 3.7 not 3.5 on Fedora 29. I assume that is the version number. Is it version dependent? When I changed the Makefile, to use 3.7 it built and installed and the result was:
$ sudo python3 test.py Count: 1 Boards: (('ASDFG', 2, 0),) Board: ('ASDFG', 2, 0) Result: 9 Result: 9 Result: 9 Result: 9
So that looked good, thought I would try a second board
$ sudo python3 test.py
Count: 2
Traceback (most recent call last):
File "test.py", line 7, in
Tried the second board by itself.
$ sudo python3 test.py Count: 1 Boards: (('PSUIS', 2, 0),) Board: ('PSUIS', 2, 0) Result: 9 Result: 9 Result: 9 Result: 9
So, I don't know if the problem is in the test program or the code???
Darryl
On Wed, Mar 6, 2019 at 2:18 AM SeanMollet notifications@github.com wrote:
Thanks! I finally saw what you were saying, I'd forgotten to change the install to use LIBDIR. I also found and fixed the malloc() bug and added a few more lib paths that debian uses on various platforms.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/27#issuecomment-469742998, or mute the thread https://github.com/notifications/unsubscribe-auth/AGcqVbHtaiaJZO-jNyk-lfJt6KA8owwjks5vTpjXgaJpZM4bXlr0 .
Sean, I have merged your code, can you look at my last message regarding the python test code.
I was a little buried last week, I'll debug that this week. Thanks!
No worries, just went very quiet.
On Mon., 18 Mar. 2019, 10:44 pm SeanMollet, notifications@github.com wrote:
I was a little buried last week, I'll debug that this week. Thanks!
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/27#issuecomment-473894918, or mute the thread https://github.com/notifications/unsubscribe-auth/AGcqVQA7QEoHXTChCtcEXdRoCIEDe5ifks5vX4o4gaJpZM4bXlr0 .
Was building for python with latest from master and ran into some compile errors:
cc -O2 -Wall -lhidapi-hidraw -L /usr/lib/x86_64-linux-gnu -Wl,-rpath /usr/lib/x86_64-linux-gnu -L./ -lusbrelay -o usbrelay usbrelay.c
/usr/bin/ld: /tmp/ccr9zx29.o: in function `main':
usbrelay.c:(.text.startup+0x62): undefined reference to `enumerate_relay_boards'
/usr/bin/ld: usbrelay.c:(.text.startup+0x1fb): undefined reference to `enumerate_relay_boards'
/usr/bin/ld: usbrelay.c:(.text.startup+0x279): undefined reference to `find_board'
/usr/bin/ld: usbrelay.c:(.text.startup+0x2c1): undefined reference to `set_serial'
/usr/bin/ld: usbrelay.c:(.text.startup+0x2fe): undefined reference to `operate_relay'
collect2: error: ld returned 1 exit status
make: *** [Makefile:37: usbrelay] Error 1
I am on Ubuntu 19
lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu Disco Dingo (development branch) Release: 19.04 Codename: disco
Installing with apt installs the CLI tool properly so no problems there. I'm not great with building but if I can help with the Make file I'll try to take a little more look.
I think it expects the library to be in place before building the python interface. Try make sudo make install sudo make install_py
On Tue., 19 Mar. 2019, 4:46 pm S, notifications@github.com wrote:
Was building for python with latest from master and ran into some compile errors:
make install_py cc -O2 -Wall -lhidapi-hidraw -L /usr/lib/x86_64-linux-gnu -Wl,-rpath /usr/lib/x86_64-linux-gnu -L./ -lusbrelay -o usbrelay usbrelay.c /usr/bin/ld: /tmp/ccr9zx29.o: in function main': usbrelay.c:(.text.startup+0x62): undefined reference to enumerate_relay_boards' /usr/bin/ld: usbrelay.c:(.text.startup+0x1fb): undefined reference to enumerate_relay_boards' /usr/bin/ld: usbrelay.c:(.text.startup+0x279): undefined reference to find_board' /usr/bin/ld: usbrelay.c:(.text.startup+0x2c1): undefined reference to set_serial' /usr/bin/ld: usbrelay.c:(.text.startup+0x2fe): undefined reference to operate_relay' collect2: error: ld returned 1 exit status make: *** [Makefile:37: usbrelay] Error 1
I am on Ubuntu 19
lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu Disco Dingo (development branch) Release: 19.04 Codename: disco
Installing with apt installs the CLI tool properly so no problems there. I'm not great with building but if I can help with the Make file I'll try to take a little more look.
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/27#issuecomment-474218422, or mute the thread https://github.com/notifications/unsubscribe-auth/AGcqVcpnQvSDOuxxp6CJDkLFi3p8FscEks5vYIe4gaJpZM4bXlr0 .
@solvire What make command did you use? Were you in the checked out folder? @darrylb123 It's searching correctly for the local library ( -L./ ), but it doesn't look like the local library has been built yet.
Sean, I've added support for a new relay type. I've also added support to refer to relays by their device. The new relays have a fixed serial so multiple boards cannot be supported any other way. I didn't change any API's and I've tested the python interface via test.py. I managed to get a seg fault once while using 4 modules and the test.py script. I haven't been able to duplicate it. If you have time, it might be worth trying python with the relay module device path. Check out the README for some documentation.
I realize this is a pretty big pull request, but there really isn't anything out there that is convenient for accessing these boards from python. Yours was the cleanest code to start from, so I used it as a base, moved the guts to a library and implemented a python extension on top of that.