On Tue, Oct 28, 2014 at 04:43:22PM +0100, Jef Driesen wrote:
I don't expect you to fix all Windows issues, but if you can already keep this in mind, that means less work afterwards.
I'll be happy to help with the Windows issues - as long as they relate to cross compiling from Linux. Maybe we can ask Lubomir to help with native compile questions...
There are also some header files like unistd.h which don't exist on windows.
I had no issues with header files when cross compiling Linus' code.
Several of the headers you include are not necessary at all, and can be removed. There are other things like the snprintf function which doesn't implement the C99 standard and returns a different value if the buffer is too short.
So what do you want done there? Is this something you will address?
I remember again what's the problem. The libusb header pulls in the windows.h header. And somewhere deep in there, there is a ERROR macro defined. I worked around that in the past by doing:
#ifdef HAVE_LIBUSB #ifdef _WIN32 #define NOGDI #endif #include <libusb-1.0/libusb.h> #endif
We don't need GDI at all, so it's no problem to disable it.
I did this for my tests and it works.
After the open call, the context pointer is stored in the device/parser structure, and you can get it from there.
Libdivecomputer doesn't log anything on its own. The actual logging is done by the application. That's on purpose. That's for example how subsurface is able to generate a libdivecomputer log directly from the GUI. I believe that's a very useful feature, which certainly doesn't sucks :-)
It may require a bit more verbosity in our code, but it saves me *a lot* of time when dealing with support issues. So it's worth the effort. Trust me, you don't want to explain some Windows user how to redirect stderr to a file. In a Windows GUI app you don't even have stderr! So this is not something I'm willing to change.
I'll be happy to rework the logging code :-)
But I can't even use that DC_STATUS_FAIL kind of thing. See later.
As to the "array_uint()" functions - you have no writing functions, and you're lacking a "u8". And your types are wrong, not allowing "void *". And they are insanely verbose. But whatever. I converted the ones you had.
Well, nobody said you may not add new functions.
Why do you need void pointers here? You're parsing a byte array, which is by definition an unsigned char array. So where do you get void pointers from?
Habit of a C developer would be my guess.
I'm not going to argue which style is better or not. There are valid arguments for both styles, and you can get things wrong anyway. If you forget to return value from read() for negative values, things can go very wrong, and so on.
The point is that I deliberately choose to not mix error codes and values in libdivecomputer. I have nothing against your implementation on its own. I'm only trying to maintain a consistent style throughout the entire codebase. If every backend starts to do things in a completely different way, then the end result becomes a mess. That's all. Nothing more, nothing less.
But the main reason why I mention the -1 in the first place is that in most places you just return -1 to indicate a failure, while in some case you could give a more meaningful error. And then we're back at the dc_status_t values, because those are the "native" libdivecomputer error codes.
BTW, one of the nice things about enums, is that whenever I see dc_status_t I immediately know it contains some status value. With plain int I need some more context. Is it success/failure (0 or -1), true/false (1 vs 0) or some value? Also in a debugger you get to see the symbolic name, not some random number.
And the horrible thing about enums is that you can't test for there existance.
Anyway, so this needs to be rewritten to comply with the libdivecomputer conventions. I can do that if Linus is too annoyed with it.
Fundamentally I'd like to know "what's the next step", "who is doing what".
I really want to see this backend in libdivecomputer (stating the obvious here), so I'm motivated to help ;-)
/D