[chirp_devel] [PATCH 5 of 5] [ft4] rework tones [#4787]

[Dan Smith]

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