[Hamlib-developer] Re: Hamlib-developer digest, Vol 1 #336 - 2 msgs

[Dale E. E. <de...@w-...>]

> Stephane,

Thanks for the quick reply as always.

>
> About your style: fix your code
> * Make it gcc-3.x friendly. Compilation should report NO warning.
>   . Especially printf(__FUNCTION__ " has been called\n"); is not accepted.
>   write instead  printf("%s has been called\n", __FUNCTION__ );
>   . Use C99 intializer
>         instead of    struct { int a; char *b } = { a: 5, b: "bad" };
>         write         struct { int a; char *b } = { .a = 5, .b = "good" };
> * Use doxygen style comments    /*<!  */
>

Most of the initializers were taken from the original kenwood.c and have
been unchanged.  I had just noticed the difference last week, but that
can be changed along with kenwood.c.

> About rig.h changes:
> * anything that has ts2k in a struct name in rig.h has no chance
>   to be merged.  (eg. ts2k_menu_t, ts2k_pm_t, etc.)
> * talking about RIG_RPT_SHIFT_1750, read my past posts

I don't know of any repeaters using the 1750 stuff around here.
It does nothing for me so I'll just drop it.  I just haven't done it yet.

>
> * ts2k menu's are ext_parms, please convert your code to ext_parms interface

When I wrote the ts2k menu's, there were no provisions.  Now there are and
I just haven't updated any of the menu stuff.

> * I dislike the "next" field in you struct channel, but I may agree on
>   some "void *priv" generic purpose field.

The reason I called it "next" is I'd like to add a function to src/rig.c.
It would be something like:
        rig_to_rig(RIG * r_dst, channel_t c_dst, RIG * r_src, channel_t c_src);

The function rig_to_rig() would call rig_get_chan(r_src, c_src)
and then rig_set_chan(r_src, c_src).

I haven't worked out any details yet but it has the potential to solve
several problems.

> * don't rename defines to your liking, you're breaking compatibility.
>   I admit some names were not choosen correctly, but stick with it.

I thought "menu" was much more appropriate than "ext", but point taken.

> * You're messing too much with RIG_VFO's.
>   I'll give you an example: RIG_VFO_AB, it's not gonna work.
>   The gnuradio backend can have many VFO, and real ones, ie. not just
>   "glorified" memories. So how would you define RIG_VFO_BF for
>   VFO_B/VFO_F?  This is broken design.

Actuall, I thought it'd be simple:

   #define     RIG_VFO_BF    RIG_CTRL_SPLIT | RIG_VFO_B | RIG_VFO_F;

but, maybe we don't care to define a zillion "uncommon" vfos so then its

        vfo_t vfo_bf;

        vfo_bf = RIG_CTRL_SPLIT | RIG_VFO_B | RIG_VFO_F;
        vfo_fb = RIG_CTRL_REV | vfo_bf;

>   What I would propose instead is something like this:
>
>   #define RIG_VFO_n(x) (1<<(x))
>   #define RIG_VFO_A RIG_VFO_n(0)
>   #define RIG_VFO_B RIG_VFO_n(1)
>   #define RIG_VFO_C RIG_VFO_n(2)

This is similar to what I've been calling the VFO Minor.
Hey, it even uses the same definition.  I like it!

>   /* the following defines as just aliases */
>   #define RIG_VFO_NONE    0
>   #define RIG_VFO_CURR    -6
>   #define RIG_VFO_MEM     -2
>   #define RIG_VFO_VFO     -3
>   #define RIG_VFO_TX      -4    /* applies to RIG_VFO_CURR */
>   #define RIG_VFO_RX      -5    /* applies to RIG_VFO_CURR */
>   #define RIG_CTRL_MAIN   -7
>   #define RIG_CTRL_SUB    -8

This is what I've been calling VFO Major.

To quote somebody that knows software better than me:
    "It's ugly, it's horrible. I want good quality."

>   My goal is to kill the band field in vfo_t. To do this, a new field
>   in rig_caps will be created to describe each VFO#, and if it is
>   exclusive with another VFO.
>   Anything else would be done using other API calls (split, weird
>   channel setup), maybe creating new ones.

This might work.  Especially if it helps understand what "state" the
rig is supposed to be in when, say, the frequency is changed.  If we
only allow RIG_VFO_CURR the function should *not* have a vfo_t
as a parameter.  What's the point?


>   Please, everyone, comment heavily on this vfo_t design. I'll try to come
>   up with a real patch this week. The vfo_t redesign is the only thing
>   holding me to release hamlib-1.1.4.
>

What about a struct with two fields:
       1) The lower order field would define the applicable VFO's (real or
fake)
             This would be the definition described above for RIG_VFO_A etc...
               and would be the bitmask (a << N);
                I refer to this as VFO minor.
        2)The higher order field would define what state the rig must be in
             for the operation being requested.  Memory, Call, Split, Rev,
etc...
              I refer to this as VFO major.

Having bitmasks available solves many questions.  "How do we specify
Reverse?"  I think this should be tied in with the vfo, not because it tells
us which PLL/VCO is being manipulated, but  tells us what is needed to set the
state external to the PLL/VCO for the proper desired operation.
VFOa (= First PLL) is used for Split, Crossband, Memory, and simplex.
If we have both pairs of data (VFOa,b, or c) and (Split, Sat, Mem, simplex)
we can move the rig from any state to any state and back again without any
loss of information or operator intent!

If we only have VFOa,b,c then we can do nothing but simplex effectively.

Functions will *not* answer this need for information.  If I want to reverse
the function of two VFOs operating as an Rx/Tx pair we say:
        vfo = RIG_CTRL_REV | RIG_SAT_UPLOAD;
        vfo = RIG_CTRL_REV | RIG_VFO_AB;
Should I write:
            rig_set_sat();
            rig_get_sat();
            rig_set_sat_rev();
            rig_get_sat_rev();
            rig_set_split();
             rig_get_split();
            rig_get_mem_split();
            rig_set_mem_split()
            rig_set_split_rev()
            rig_get_split_rev()
            rig_get_crossband()
            rig_set_crossband()
            rig_set_mem_split_rev()
            rig_get_mem_split_rev()

            and the list goes on....

We need the information even if we call rig_set_split();
Is it A/B or B/A?  Having the bitmask answers every
 question we may have.

int mmelter_set_split(RIG *rig, vfo_t vfo)
{
    ...
   if( vfo & RIG_CTRL_REV) {
            // do B/A or C/A or ... or Z/F or ...
    else
            // do A/B or A/C or ... or F/Z or ...
    ...
    return RIG_OK;
}

Yes, we'll almost certainly want to call a few functions
for special modes.  Having called that special function and
mode, we need to know what to do with it!

> Note about CVS repository:
> * please, no autogenerated file (eg Makefile.in, ..) in the CVS base.
> * also, if you're not modifying a HEAD file, do NOT duplicate and
>   check it in your branch! Otherwise, you drift from HEAD, and don't
>   get updated in case of new changes (which was the case).

Somethings wrong with the branch then.  It's probably linked back
to real early releases because from the start I've never gotten the current
hamlib.  It becomes more stagnant all the time.

> > 2) Also, I'm working on a command-line.  You can glance at it and see
> > if you think its going the right direction.  I'd eventually like to have
> [..]
>
> Sorry, I haven't had time to look at it. Fix the 1) first, you have a
> lot to do.

This is just a hobby I do because I really enjoy it.  I'm interested mostly
in the command line.  However, it will have to take the backburner for
a little while.


73's

Dale, kd7eni