[chirp_devel] 03/07 Build Release

[Dan Smith]

> Dude, where is the IC-7300? I have viscous hordes clamoring for it.
> Pitchforks, torches, the works....

I laid out some of my concerns with this the last time:

http://intrepid.danplanet.com/pipermail/chirp_devel/2020-October/006078.html

including the reimplementation concern, which was shared with at least one other person here:

http://intrepid.danplanet.com/pipermail/chirp_devel/2020-October/006088.html

You should know from our TM-D710 interaction that I'm highly concerned with file format stability for things like this, and as you noted, you're loosely constructing one on the fly. Looking back on the archives, I probably wasn't overtly forward enough about that fact in this exact context, but I didn't see much of a constructive response from you to the concerns that _were_ outlined. Thus, I had assumed you were dropping it. If you want to work towards something that I think is acceptable, we can surely talk about it.

Let me try to summarize the most important points:

My primary concern is that you are creating a new binary file format that is not constrained by the layout of the radio, but rather by the order you're pulling things out of the radio (and translating them on the fly). The reason I'm concerned about that is because I don't trust that you/we will be rigorous enough to keep that compatible forever without some definition, tests, etc. Breaking compatibility with a file format is the worst thing we can do to the users, IMHO, well above any sort of UX concerns. If a user saves an ic7300 file today and then tries to open it next year with whatever chirp is current at the time and we either (a) fail to open it or (b) load garbage because the format has changed and we didn't notice, then that is game over in my eyes. I want to avoid that at all costs. More on this in a minute.

My second concern is the fact that this reimplements a lot of stuff in the icom live code instead of re-using it. You're doing effectively the same thing in your code, but with different intent. That code is structured to return memories live from the radio, but you want to load them all at once. I would be a lot happier if you would just use that code, even though you're implementing a clone driver. It should be totally reasonable to, in your clone code, spawn an instance of a live icom driver and just get_memory() in a loop until you have them all. That would avoid re-implementing all the over-the-wire stuff (which so far has worked for all CIV radios that I know of) and leave the rest of us with fewer lines of code to maintain. If there's something we need to change/extend in the live code to handle the new-ness of the 7300, then that's good to know and we should do that, and hopefully we get a live-mode 7300 driver out of the deal for almost free. Please look at this as leaving us all on the hook for your code if you get hit by a bus the day after I put your driver in the tree: we will miss you, but not appreciate having a second implementation of a complex protocol that need not be duplicated. Addressing the problem this way would also eliminate some of the quality concerns I detailed in those routines.

Related to both of the above, I don't like that you're translating the data in between reading it from the radio and storing it, as that leaves too much room for the file format breakage I'm talking about. I'd feel a lot better about if if you defined rules at the top that say something like this:

 "The file format of this is NNN whole channel frames, followed by M settings frames from address X to Y"

...and stuck to that as what gets stored on the disk. Parse that when you load (a la memmap) but be faithful in storing the exact contents of the radio in the sequence you define, as if you didn't get to define it. That's the kind of rigor I'm looking for to have confidence that we're going to be able to keep this stable long-term, and is the same thing I wanted from the TM-D710 work.

As far as the general approach, I really don't understand the desire to force a live radio into clone mode, as I don't see it as a benefit. If I want to edit my memories offline, I'll export to a CSV file. With the TM-D710, not everything was available through the live interface, so adding the clone mode driver made sense as it opened up more possibilities. That's not the case here. As noted in the previous discussion, you don't have to wait for all memories to load from the radio to do anything (you can even interrupt the load and define a window of memories you want to load immediately), changes interrupt the load and happen immediately, the driver ends up self-checking because the radio will refuse invalid things, and settings support is totally doable for live radios. The only thing I can really think of that doesn't work well with live mode radios is the use-case of cloning the same image into multiple radios. That's likely quite uncommon for the CIV radios, and won't work anyway unless you implement every last block the radios supports.

That said, I also don't want to force my workflow on other people, as long as having multiple options doesn't cause other problems (as detailed above). I'd ask the rest of the developers here who have an opinion to chime in and answer the following:

  Do you think it's worth us having this synthetic live-but-clone-mode
  driver in tree (assuming the above is addressed) or would you rather
  stick to live mode radios having the live behavior and not muddying
  the water without a compelling reason?

If people want the driver to behave like a clone mode radio, then I won't stand in the way, but I will insist on addressing the above design points. I'd definitely welcome some other voices here, so if you have an opinion, please speak up.

--Dan