> + "->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@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@intrepid.danplanet.com
http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers