Re: [Hamlib-developer] branch_ts2k

[Stephane F. <f8...@fr...>]

On Sat, Mar 01, 2003, Dale E. Edmons wrote:
> 1) If you have some time in the near future could you check-out and
> look at  branch_ts2k?  Especially the include/hamlib/rig.h and how
> I added #define's with hooks into rig_newvfo.h.  This would allow
> minimal changes to rig.h and prevents the ts2k stuff from interfering.

The rig_newvfo.h has stricly no chance to be merged with the HEAD.
It's ugly, it's horrible. I want good quality.

Everything that belongs to rig.h should be in rig.h.
I want patches, not a rewrite of the API.
Also keep in mind that it has to be source level compatible, 
if not binary level compatible.

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	/*<!  */

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
* ts2k menu's are ext_parms, please convert your code to ext_parms interface
* RIG_FUNC_TSQL and rig_set_ctcss_sql are NOT duplicate! read pas posts
* I dislike the "next" field in you struct channel, but I may agree on
  some "void *priv" generic purpose field.
* don't rename defines to your liking, you're breaking compatibility.
  I admit some names were not choosen correctly, but stick with it.
  Example are: RIG_MODE_WIDE, RIG_MODE_NARROW, tone, dcs, etc.
* 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.
  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)
  /* 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

  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.
* Anything that's not a VFO is something else, most probably a kind of
  memory channel. Memory channel numbers can be forged, using negative
  value for instance.
  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.

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).

> 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.


Stephane