libpktriggercord #49
Comments
|
I'll create a new branch for this development: https://github.com/asalamon74/pktriggercord/tree/libpktriggercord we can freely commit there, at the very end I just squash the commits to the master branch. So far I added the commit (I had to rebase it) uploaded in #48 and also added commits ( karlrees#1 ) by@blemasle . |
|
About the packaging question: Probably the real nice solution would be to create a libpktriggercord package and a separate pktriggercord package, the latter should require the first. So if someone want's to use it only as a library, no need the install both packages. On the other hand it would make more difficult to install the downloaded rpms, so for now I think we can just use one single package. Later we can revisit this decision. |
|
Right now windows cross-compilation does not work, this is also reported by travis: https://travis-ci.org/github/asalamon74/pktriggercord/builds/679689138 |
|
I agree that that it would probably be best in the long run to separate the packages. It'll probably take a while to get there, though. Not sure how much time I'll have in the immediate future to contribute, but I'm happy to provide input. I may have some time later next month to actually code. |
|
Great news !
Just tried under Ubuntu 20.04 LTS, same issue. I never face this when running I'm currently trying to setup an dev environment similar to the one used on Travis CI (Ubuntu Xenial 16.04) so I can get the whole thing to compile and work on the windows cross compilation. |
|
Finally got the whole thing to compile and run under a Xenial VM. Using any distro more recent than that just crash with a segfault. For the moment, I just fixed the windows compilation and nothing else. We should start to move code around so that both the cli and the gui use code defined within the lib, even if the "lib" is a simple .c file at first. At the moment some functions extracted from the cli (notably I'm happy to start working on it, but it could mean undoing what @karlrees did for the new pentax INDI driver. Not permanently, but just the time to figure what goes in the lib and what needs to be changed in the cli/gui to reuse that code. |
|
I'm fine with that. I figure once we get the library standardized, I'd update the INDI driver to whatever API we decide upon anyway. I was thinking we should put this all in a separate namespace, but then I remembered it's C instead of C++. So we should probably append all of the exposed functions with a unique prefix to avoid conflicts with other libraries. Maybe pk_? |
|
The current code base already use It will also make the code easier to port to the "new" library for those that currently use a forked version of this repo (eg Gphotos), because renaming will be limited. There is also a recurring BTW : fixed android build :) |
|
PkRemote (the project I forked to create PkTriggerCord) had a libpslr library (https://sourceforge.net/p/pkremote/code/HEAD/tree/Makefile) so I think the original idea was to put In a nice world these should not be in the same file, but it was already mixed. So probably the easiest is to use Travis check is green, yeah. :) |
|
Unfortunately we have two |
|
Great ! Thanks for all the info. FYI, I'm currently working on making a static libpktriggercord static lib that the cli reuses. This involves some changes and reorganization in the Makefile and as it's probably breaking the build I won't push it for the moment. |
|
If things break, I'll try to fix it along the way. libpktriggercord.aAll .c/.h "core" files now build to a static library libpktriggercord.so / libpktriggercord.dllThe shared library is built from pktriggercord-cli / pktriggercordBoth applications are linked to the static library. MakefileI've reworked the Makefile to remove redondant code & configuration. Appart from the downlod of gtk2 binaries, I've entirely removed the win32 compilation that was using an outdated mingw32 (the so called Cross compilationTargets have been reoganized / redefined so that Linux & Win32 targets are identical, minus a few configuration differences (and the download of the gtk2 binaries of course). |
|
Originally I created the cross-compilation ( Building both platforms at the same time is not important, I never trusted the build system that much anyway. I modified the Makefile a bit, now targets like I also removed the One thing is a bit still strange in the windows cross-compilation: We still have the win_dlls library. There are the dlls from mingw32 required for the windows build to run. In the long run this directory should be eliminated. |
I hesitated to do so, but didn't because I wanted to limit conditionnal code. Ideally we should use a configure (or even better, CMake as @karlrees has already done all the hard work ?) to make cross platform and dependencies easier 😃 But one thing at a time.
Maybe they could be downloaded at build time in the same way the gtk dlls are ? |
|
Yes, there are quite a few tools (configure, cmake, ...) for this purpose, but a simple (?) Makefile is working right now, and it's really portable. Probably downloading the dlls are better, I'll check it. |
|
I just pushed a bunch of renaming we've talked about. This renaming mostly concern symbols that will most likely be exported; For reference, here there are.
I'll try to keep this list up to date is other renaming are performed, it could be useful once the library get published. |
|
Awesome. Wish I had some time to test things. As for the cmake discussion, I was only getting cmake to compile my version of the library on linux, I never really got it doing anything else. |
|
I'm currently trying to find the best and most maintainable way to restrict exported symbols, but I'm afraid this would mean a complete recompilation for the .dll. There will no longer be an option to create the .dll from the static lib, because we need different attributes placed on symbols in each case. The issue would only be for the .dll though, as .so works diffrently and will probably not need recompilation. |
|
Only relevant symbols are now exported when building the lib. I however noticed that the gui isn't running anymore since I guess b27ca63 because of missing dlls :
|
|
I haven't got much time to work on this on the past few weeks, but things are clearing out right now. My next step : Moving things around so that the cli/gui can be built against the shared lib. I'm aware it is not what we are looking for in the end. But if it can be done, then libpktriggercord is most certainly approaching the point where it is ready. Trying do to so has already pinpointed some hiccups in the exported symbols :) |
|
Win32 dlls dependency hell is solved once and for all (hopefully) by using the gtk bundle available here : There is no additional download needed. This ensure that output are compiled against the same source as they are dynamically linked at runtime (the bundle contains src, lib and binary files). With these changes, win_dlls can be removed entirely. Some symbols were also missing from the original work. This is now fixed. |
|
Sorry I had no time for this project in the last moth. Somehow github commit page does not show the travis checks, but it looks good for this branch: https://travis-ci.org/github/asalamon74/pktriggercord/builds/703299876 Downloading one bundle zip is a much cleaner solution. Great idea. |
|
No worries, I myself am only dedicated to this when I have some spare time :) Anyway, it is much simpler indeed, and now you can also be sure that what has been used to build the app is the same code as the packaged dlls 👍 I think we should start to consider a merge to master, before breaking things further. For one, it could provides a smoother transition from forked code to library usage. Further refactoring (bulb for instance) has nothing to do with the library itself and should be done separately. I also think we should wait for a new release with the few recent fixes before merging. Doing so will allow any fork to include the latest fixes easily before a more tedious work. But there is no rush :) I'm just not sure where to put .def and .lib files. Should they really go with the win32 distribution package ? |
|
Hi @asalamon74 , As I didn't hear for you after the last commits, I left the project aside. Today I merged the last fixes from master to the branch. I'll open a pull request with it. Even if it's not get merged right away, it will make things easier to track. Cheers. |
|
@blemasle Sorry for the missing response. This is a big change and I was always afraid to commit it and had no time to really testing it. This change addresses multiple things so I think I'll try to commit in multiple parts. I'll start with the windows compilation changes which are quite useful even without the library part. Thanks for all the work. |
|
@asalamon74 No worries. Appart from a massive renaming / moving, there isn't much change to the code itself. |
|
Great! Do you still plan to reintegrate the branch at some point in the future? I don't mind if you don't, really. I think it would be bad news for other pieces of codes out there that have to maintain a fork of this repository for the exact purpose this branch was set up initially. But I in total honesty don't want to spend time maintaining that branch up to date with master if it won't be merged. Every change committed to master needs special care because the code has been moved around so that it could be included in the library. Regards |
|
One more commit, the method renames: bdba056 I'll keep adding more and more from this branch. I know that I'm terribly slow, but it's getting closer: |
No worries. I'm glad to see that this work will eventually be reintegrated at some point 👍 If you ever need help to glue old and new commits together, do not hesitate to reach me :) I remember having a hard time moving functions around so that references were optimal for instances (no extra function in the lib etc). |
|
And one more commit, the logging changes: 5f32c9e This is again a useful change on its own. |
|
One more commit for pslr_utils: 008737c |
A pktriggercord libary could make it easier to include pktriggercord into other programs.
This is mentioned here: indilib/indi-3rdparty#66
and the first version of thelibrary was uploaded by @karlrees in #48
The text was updated successfully, but these errors were encountered: