Open srcejon opened 4 years ago
I've updated the patch to include:
Support playing audio files on AOS and LOS. Issue #187. Allow elevation for AOS and LOS to be set to something other than 0 degrees. Issue #167. Signal LOS when autotrack is enabled. Issue #193. Update Windows makefile to work with latest version of msys2. Issue #110.
Another update allows applications (shell commands) to be run on AOS and LOS as well (perhaps to run a decoder, or control some other h/w), as requested on the forum.
Note, on Windows, if you want the command to run asynchronously, prefix it with start. On linux, add & at the end.
Thanks for the patch and sorry for the late reply. Unfortunately, I'm busy with other things at the moment but I will take a look at it as soon as I can. However, there is a large backlog of critical bugs that will need my attention before adding new features.
I took a very brief glance and I can see that you have pushed 4 different feature additions into one pull request, resulting in a patch that adds 943 lines of code. That's not good! Please submit separate PRs for each new feature and bug fix so that they can be reviewed and tested individually.
Thanks, and again sorry for not being able to process these at the moment.
Alex
Hi Alex.
These are all on one branch because they all touch the same bits of code, but are addressed seperately in each commit, so you can see (and merge) individual diffs here: https://github.com/csete/gpredict/pull/192/commits - You'd get merge conflicts if they were each on a separate branch. (The other changes I've made (for ground tracks etc) are on a separate branch, as they are independent.)
There are already existing PRs for what is covered, referenced above. #167, #182, #187, #193. I've also just added #198.
I know very well how git and github works, and I also know that even if I did as you suggest it wouldn't change the fact that commit d391ce92ff0e3120a5273e3c70b047e4f7b33d45 addresses four separate issues, three bugfixes and a new feature that plays audio. By the way, that last part is a show stopper for the whole thing because I am not going to accept GStreamer as a dependency for playing audio files.
Sorry, but I can not review this in its present form.
gstreamer is added as an optional package - so it can be built without it. Is there a better cross platform library? gstreamer built for both Linux and windows successfully.
My primary concern with gstreamer as a dependency is that it is not just a simple library but a whole framework for audio and video processing and therefore it feels like overkill for just playing audio. As far as I recall, the library only provides the framework, then it needs plugins for pretty much everything like audio interfacing, file loading, etc. And all those plugins probably have their own dependencies and every single dependency adds complexity when it comes to distribution, testing, and maintenance.
Anyhow, gpredict is now soon 20 years old and there is a lot of bug fixing, refactoring, and finishing partially implemented features that need to be done before new features can have any priority. The next couple of releases will most likely be maintenance releases that address these issues.
This patch adds the ability for a user to define a ; separated list of rigctrld commands to be sent to the radio on AOS or LOS. E.g. set_powerstat 0/1 to turn on/off the radio.
It also adds the ability to send a user-defined command when tuning to a transponder. This allows the appropriate mode, bandwidth and antenna to be set automatically. E.g. set_mode FM, 10000 ; set_ant 0
The AOS/LOS commands can be defined in the GUI. Currently, the transponder commands need to be defined in the .trsp file, with the COMMAND key. E.g:
33591.trsp [APT Downlink] DOWN_LOW=137100000 MODE=APT COMMAND=set_mode WFM, 40000;set_ant 0
Eventually this should be added to the GUI, but I thought I'd see if this patch was agreed to first.
This feature was requested in Issue #182