Repository navigation
Hamlib rotator support - #455
Conversation
|
It works for me, but there's a few open ends:
Comments, feedback? |
|
|
||
| if (pfx_index != my.countrynr && 0 == get_qrb(&range, &bearing)) { | ||
|
|
||
| pthread_mutex_lock(&tlf_rot_mutex); |
There was a problem hiding this comment.
do we need a mutex for rot?
rig controls are called from both threads (main + background), but rot looks to be called from main only.
another question: does rot_set_position() just send a message to the rotator and doesn't wait for it to actually reach to requested position?
There was a problem hiding this comment.
We probably don't need the mutex now, but I would think it's safer to include it. New code might violate the assumption.
set_position just starts the rotation. You wouldn't want the rest of the program to halt for a minute while the antenna is turning...
Thanks for the review, will fix the variable init later.
There was a problem hiding this comment.
Do hamlib guarantee that you can call functions for rig control and rotator control at the same time?
Otherwise (as set_position comes back immediately) why not use tlf_rig_mutex for both?
There was a problem hiding this comment.
rig control and rotator control are talking to different devices, so calling functions at the same time is not a problem.
We could use the same mutex, but... this feels wrong.
There was a problem hiding this comment.
Fwiw the next step would be to read out the bearing and show it in the UI (I get reminded about this every time I'm using the station from remote), there the mutex might already be required.
|
|
||
| if (pfx_index != my.countrynr && 0 == get_qrb(&range, &bearing)) { | ||
|
|
||
| pthread_mutex_lock(&tlf_rot_mutex); |
There was a problem hiding this comment.
Do hamlib guarantee that you can call functions for rig control and rotator control at the same time?
Otherwise (as set_position comes back immediately) why not use tlf_rig_mutex for both?
|
Can you please check, where the delete of test/test_rules.c comes from? |
|
Sorry, I should have pushed this to a different branch... |
|
I think this is good to go - these are the changes I've been using over the past weeks. If people agree, I'll do the final polishing and push a rebased version. |
Looks mostly good. But can you please have a look at the failing tests? Thanks |
8408dfa to
46afb1a
Compare
|
Sorry that it took so long, I always pick this up shortly before a contest, and after a weekend on the air, time is running out to actually submit it. Now I started a bit earlier... I added the ability to rotate to the long-path direction. Since I was never really happy with Ctrl-D, keys are Since tests were not happy with hamlib code being in qrb.c, I moved the rotator interface to where the rig interface is living. sendqrg.c doesn't really fit (did it ever fit the rig interface?), but now similar code is in a similar place. Tests are passing now. I think I addressed all concerns, IMHO the code is good to go now. |
|
Fwiw, I've been using this version of the rotator support in the last contests and for some casual operation. The |
|
Sorry, for late comment - was afk for some time. |
|
I finally found the time to do the updates. Sorry for being so slow... |
|
Time is sparse atm. Hope to find time to look into it in coming week. Sorry. |
|
|
||
| /*-------------------------------------rigctl-------------------------------*/ | ||
| char rotconf[80]; | ||
| #ifdef HAVE_LIBHAMLIB |
There was a problem hiding this comment.
Hamlib is mandatory, please drop both checks. It just hasn't been cleaned up.
| #endif | ||
|
|
||
| {"RIGPORT", CFG_STRING_NOCHOMP(rigportname)}, | ||
| {"ROTPORT", CFG_STRING_NOCHOMP(rotportname)}, |
There was a problem hiding this comment.
CFG_STRING_NOCHOMP is used for RIGPORT due to some backwards compatibility (I'll revisit this, see below). Please use plain (chomping) CFG_STRING.
parse_logcfg.h:
// FIXME: remove NOCHOMPs
There was a problem hiding this comment.
Changed, and updated the test file for the new (better) value.
| {"import", 'i', 0, 0, "Import Cabrillo file to Tlf format"}, | ||
| {"no-cluster", 'n', 0, 0, "Start without cluster hookup" }, | ||
| {"no-rig", 'r', 0, 0, "Start without radio control" }, | ||
| {"no-rotator", 'R', 0, 0, "Start without radio control" }, |
|
At the moment I see only one point to think about: It may make sense for large countries, but how to find the correct direction by program? |
|
I just pushed a new version addressing @zcsahok's comments, thanks for the review!
My next idea was to add "rotate 30° left/right buttons" (for example on Other future ideas might be:
|
The sounds reasonable. As a side note: I fear that it will be difficult to find a lot of unused single key codes for additional rotator commands. As it probably will not be used very often a two key combination with a common first key may be appropriate (like '.' for filtering the multi infos).
At least the first point will be very useful. |
This adds rotator support. Configuration is done exactly like rig support with ROTATOR_CONTROL, ROTMODEL, ROTSPEED and ROTPORT. Two new keybindings are introduced: ^ (think "arrow") rotates towards the country of the current call, and & (the key right of ^ on US keyboards) rotates to the long-path direction. If the current call is from the own country, no rotation is done.
|
Rebased. |
|
Thanks for the work Christoph |
|
Thanks for the reviews! |
This adds rotator support. Configuration is done exactly like rig support with ROTATOR_CONTROL, ROTMODEL, ROTSPEED and ROTPORT. Two new keybindings are introduced: ^ (think "arrow") rotates towards the country of the current call, and & (the key right of ^ on US keyboards) rotates to the long-path direction. If the current call is from the own country, no rotation is done.
This adds Hamlib rotator support to tlf. When a call or prefix is entered, pressing Ctrl-D rotates the antenna to the calculated bearing.