[chirp_devel] [PATCH 5 of 5] [ft4] rework tones [#4787]
Dan Clemmensen
Thu Feb 28 08:29:56 PST 2019
I thought that I had run cpep8 after the last patch, at least., made
changes to eliminate all cpep8 errors.
On Thu, Feb 28, 2019 at 8:27 AM Dan Clemmensen <danclemmensen at gmail.com>
wrote:
> > + "->Tone": ("R-TONE", None),
>
> I had a question about this in the previous patch.
>
> > + "Tone->Tone": ("TSQL", None)
> > + }
>
> These are two mappings from the CHIRP definitions to the Yaesu
> definitions. Yaesu uses "TSQL" to mean "send and receive ctcss tones,
> and the toned do jot need to be identical. thus we must map to "Yaesu
> TSQL" on both cases. I chose to use the name "TSQL" for to this
> Yaseu value because it's in the Yasu documentation and it's what you see
> on the radio's screen. I could have renamed it in the ptyhon
> code, but the other drivers seem to adhere to the manufacturer's
> documentation.
>
> Sinmilarly, Yaesu "R-TONE" is the name for "don't send a tone, but do
> receive a tone." The Yaseu name for reverse polarity tone squelch is "REV
> TN".
>
>
> About the use of a single table: I have been thinking about that a lot.
> It's not really a trivial problem, but I think that may be a reasonable
> solution.
> the underlying problem is that an abstract radio can perform some or all
> of 15 combinations of tone handling. :
> Tx (none, ctcss,dcs) x Rx (none, ctcss,dcs,ctcss-R, dcs-R)
> and of these two two are split (ctcss==, ctcss!=) and (dcs==, dcs!=). The
> CHIRP mem structure uses two fields to represent this and each
> radio uses its own strange multi-field encoding.
>
> To complicate this, I originally thought that the CHIRP unit tests require
> that we preserve tone fields even some cases where a given
> tone field is clearly a "don't care" as far as the radio is concerned.
> This is a burden on the code and on the approach we must take.
> I now think I was not quite right about this,and it turns out that could
> simply not put any value into the mem structure for these cases.
>
> None of the other drives I looked at even attempt to create an explicit
> two-way mapping. Instead, they they use one if-test chain for encode and
> a second if-test chain for decode. When I originally wrote my code by
> example, I simply converted each of these if-chains into a separate table.
> The code has substantially different requirements for the two tables, so
> Idon't think that there is a single table that is both comprehensible and
> has simple lookups for both directions. a simplified table will
> complicate the code. I'll give this another try today.
>
> On Thu, Feb 28, 2019 at 7:12 AM Dan Smith via chirp_devel <
> chirp_devel at intrepid.danplanet.com> wrote:
>
>> > @@ -217,7 +216,7 @@
>> > return sum(x for x in bytearray(data)) & 0xFF
>> >
>> >
>> > -def variable_len_resp(pipe):
>> > +def variable_len_resp(pipe, cmd):
>> > """
>> > when length of expected reply is not known, read byte at a time
>> > until the ack character is found.
>> > @@ -233,8 +232,11 @@
>> > response += b
>> > i += 1
>> > if i > toolong:
>> > - LOG.debug("Response too long. got" +
>> util.hexprint(response))
>> > - raise errors.RadioError("Response too long.")
>> > + msg = "received too many bytes from radio before ACK. "
>> > + msg += "Sent " + util.hexprint(cmd)
>> > + msg += ". Got " + util.hexprint(response)
>> > + LOG.debug(msg)
>> > + raise errors.RadioError(msg)
>>
>> Heh, okay, I see I jumped the gun here. However, hexprinted debug is way
>> too much info to show to the user too. Somewhere in the middle please. I'm
>> sure I'm seeming overly picky here, but we have some variety in the error
>> messages that people get since they're all defined by the deep internals of
>> the drivers, so I'm trying to steer this somewhere towards the center.
>>
>> I can also change this up to be what I think it should be (like the
>> examples I gave earlier) if you want.
>>
>> > +# map a CHIRP tone mode to a FT-4 sql and which if any code to set to
>> 0.
>> > +# The keys are provided to RadioFeatures as a list, so these are the
>> > +# only values we expect to see in mem.tmode. This map as the identical
>> > +# structure as the one below to simplify the code. It permits stricter
>> > +# zeroing of the unused fields later if we find that it is needed,
>> > +# but the CHIRP unit tests want us to leave them alone.
>> > +MODES_TONE = {
>> > + "": ("off", None),
>> > + "Tone": ("T-TONE", None), # chirp means "xmit, not rcv"
>> > + "TSQL": ("TSQL", None), # chirp means "xmit and rcv,
>> tx==rx"
>> > + "DTCS": ("DCS", None), # chirp means "xmt and rcv,
>> tx==rx"
>> > + "TSQL-R": ("REV-TN", None), # chirp means reverse R-Tone
>> > + "Cross": () # not used in lookup, needed in
>> list
>> > + }
>> > +
>> > +# Map a CHIRP Cross type if the CHIRP mem.tmode is "Cross". The keys
>> > +# are provided to RadioFeatures as a list, so these are the
>> > +# only values we expect to see in mem.cross.
>> > +MODES_CROSS = {
>> > + "DTCS->": ("DCS", "rx_dcs"),
>> > + "->DTCS": ("DCS", "tx_dcs"),
>> > + "DTCS->DTCS": ("DCS", None),
>> > + "->Tone": ("R-TONE", None),
>>
>> I had a question about this in the previous patch.
>>
>> > + "Tone->Tone": ("TSQL", None)
>> > + }
>>
>> Tone->Tone is only for when the tones are different, which is not what
>> TSQL means...
>>
>> > # names for the setmode function for the programmable keys. Mode
>> zero means
>> > # that the key is programmed for a memory not a setmode.
>> > SETMODES = [
>> > - "mem", "apo", "ar bep", "ar int", "beclo", #00-04
>> > - "beep", "bell", "cw id", "cw wrt", "dc vlt", #05-09
>> > - "dcs cod", "dt dly", "dt set", "dtc spd", "edg.bep", #10-14
>> > - "lamp", "led.bsy", "led.tx", "lock", "m/t-cl", #15-19
>> > - "mem.del", "mem.tag", "pag.abk", "pag.cdr", "pag.cdt", #20-24
>> > - "pri.rvt", "pswd", "pswdwt", "rf sql", "rpt.ars", #25-29
>> > - "rpt.frq", "rpt.sft", "rxsave", "scn.lmp", "scn.rsm", #30-34
>> > - "skip", "sql.typ", "step", "tn frq", "tot", #35-39
>> > - "tx pwr", "tx save", "vfo.spl", "vox", "wfm.rcv", #40-44
>> > - "w/n.dev", "wx.alert" #45-46
>> > + "mem", "apo", "ar bep", "ar int", "beclo", # 00-04
>> > + "beep", "bell", "cw id", "cw wrt", "dc vlt", # 05-09
>> > + "dcs cod", "dt dly", "dt set", "dtc spd", "edg.bep", # 10-14
>> > + "lamp", "led.bsy", "led.tx", "lock", "m/t-cl", # 15-19
>> > + "mem.del", "mem.tag", "pag.abk", "pag.cdr", "pag.cdt", # 20-24
>> > + "pri.rvt", "pswd", "pswdwt", "rf sql", "rpt.ars", # 25-29
>> > + "rpt.frq", "rpt.sft", "rxsave", "scn.lmp", "scn.rsm", # 30-34
>> > + "skip", "sql.typ", "step", "tn frq", "tot", # 35-39
>> > + "tx pwr", "tx save", "vfo.spl", "vox", "wfm.rcv", # 40-44
>> > + "w/n.dev", "wx.alert" # 45-46
>>
>> Did this change or did you just reformat it? What about the other copy of
>> this you have in the file?
>>
>> > class YaesuFT65Radio(YaesuSC35GenericRadio):
>> > @@ -1052,9 +1070,9 @@
>> > MAX_MEM_SLOT = 200
>> > Pkeys = 4 # number of programmable keys on the FT-65
>> > namelen = 8 # length of the mem name display on the FT-65 front
>> panel
>> > - id_str=b'IH-420\x00\x00\x00V100\x00\x00'
>> > + id_str = b'IH-420\x00\x00\x00V100\x00\x00'
>> > legal_steps = list(STEP_CODE)
>> > - legal_steps.remove(6.25) #should not remove if euro version
>> > + legal_steps.remove(6.25) # should not remove if euro version
>> > # names for the setmode function for the programmable keys. Mode
>> zero means
>> > # that the key is programmed for a memory not a setmode.
>> > SETMODES = [
>> > @@ -1063,10 +1081,10 @@
>> > "dc volt", "dcs code", "dtmf set", "dtmf wrt", "edg bep", #
>> 10-14
>> > "key lock", "lamp", "ledbsy", "mem del", "mon/t-cl", #
>> 15-19
>> > "name tag", "pager", "password", "pri.rvt", "repeater", #
>> 20-24
>> > - "resume", "rf.sql", "scn.lamp", "skip", "sql type", #
>> 25-29
>> > + "resume", "rf.sql", "scn.lamp", "skip", "sql type", #
>> 25-29
>> > "step", "tot", "tx pwr", "tx save", "vfo.spl", #
>> 30-34
>> > "vox", "wfm.rcv", "wide/nar", "wx alert", "scramble" #
>> 35-39
>> > ]
>> > +
>> > def __init__(self):
>> > self.add_paramdesc("misc", ("compander", "Compander", ["ON",
>> "OFF"]))
>> > -
>>
>> Looks like some whitespace changes jumped in here :)
>>
>> I've applied the first four in this set and would like to see some
>> discussion of the previous items I have before we proceed on this one.
>>
>> I snuck a change in between these to add ft4.py to the cpep8.manifest so
>> that style checks always run on the file, and it looks like there are still
>> a bunch of offenses in there. Maybe we can get that fixed up shortly in
>> follow-up?
>>
>> Thanks!
>>
>> --Dan
>>
>> _______________________________________________
>> chirp_devel mailing list
>> chirp_devel at intrepid.danplanet.com
>> http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
>> Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20190228/040dd8d7/attachment-0001.html
More information about the chirp_devel
mailing list