dl5di / OpenDV

Open Digital Voice software for Amateur Radio based on Jonathan Naylor's (G4KLX) "ircDDBGateway" and "PCRepeaterController" for D-Star
GNU General Public License v2.0
106 stars 63 forks source link

Cleaning up the GMSK Modem LibUSB dependencies #66

Closed mcdermj closed 8 years ago

mcdermj commented 8 years ago

There are a lot of files in the tree with huge #ifdefs around the entire file. This isn't really optimal as it doesn't allow much code sharing among the ports and causes things to slip from being in sync and getting fixed. This file is one of those that shares a lot of commonality but has redundant code. This patch separates out the things that are truly different and puts them in separate preprocessor conditionals.

That being said, there is a WinUSB piece for Windows users which, I assume, is preferred over using LibUSB on that platform. Especially since the URI and K8055 drivers only use WinUSB on Windows. If that's the case, we probably shouldn't even give Windows users a choice and make this a Linux-only file and drop it from the Windows build project files. Otherwise, this targets the libusb-0.1 library and probably should be updated to use the libusb-1.0 library which is available on Windows now in the main tree.

g4klx commented 8 years ago

Please leave libusb as an option on Windows. The whole USB GMSK Modem thing is really painful and I spent an extremely long time on it, and libusb worked better for some people and WinUSB for others. I'd love to drop the whole GMSK Modem support but that's not possible.

mcdermj commented 8 years ago

To be clear, this patch doesn't remove any options for any platforms.

What it does do is make more clear what the differences are between platforms by only #ifdef-ing individual changes rather than essentially having two copies of the file in one. This allows the code sharing between platforms to be explicit. For example, the readHeader function is nearly the same between platforms. The only difference is accounting for the difference between the usb_strerror used in LibUSB 0.1 and the libusb_error_name function used in LibUSB 1.0. I abstracted these into an inline libUsbLogError function that can be called by both platforms. This makes the function exactly the same between the two platforms so that if there are bug fixes in one, it'll get automatically picked up by the other, because it's the same code.

Things like this are all this patch really does. It's probably easier to see these changes by looking at the final result rather than the diffs.

F4FXL commented 8 years ago

Hi,

This patch broke Windows build ..... :(

F4FXL commented 8 years ago

libUsbLogError does not build, the function pointer is not accessible some function pointers have signature missmatches... Can I suggest you install VS2015 Community edition and try to fix it ?

F4FXL commented 8 years ago

Fixed https://github.com/dl5di/OpenDV/pull/69

mcdermj commented 8 years ago

I'm working on getting an environment up and running just to make sure I don't break things. I'm thinking that what might help solve this on a longer term basis is to maybe start doing some Continuous Integration stuff with both Linux and Windows. That way we can keep track of build breakage a lot better. Maybe even have a staging tree before things get pulled into master? I already have a Jenkins server running and some resources I could put together to support this if people are interested.

F4FXL commented 8 years ago

Hi Jeremy,

This is a good idea, I was alredy thinking about it but had not time to invest into it. At my daytime work we use Jenkins. Are there any free CI providers we can use ?

I'm working on getting an environment up and running just to make sure I don't break things. I'm thinking that what might help solve this on a longer term basis is to maybe start doing some Continuous Integration stuff with both Linux and Windows. That way we can keep track of build breakage a lot better. Maybe even have a staging tree before things get pulled into master? I already have a Jenkins server running and some resources I could put together to support this if people are interested.

Reply to this email directly or view it on GitHub.