[chirp_devel] 03/07 Build Release
Dude, where is the IC-7300? I have viscous hordes clamoring for it. Pitchforks, torches, the works....
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
Dan Smith via chirp_devel chirp_devel@intrepid.danplanet.com writes:
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).
Perhaps we should be avoiding binary formats entirely, and store things in json instead. Yes, it's a big bigger, but I doubt that matters, compared to it being at least somewhat self-describing and nerd-readable.
73 de n1dam
Perhaps we should be avoiding binary formats entirely, and store things in json instead. Yes, it's a big bigger, but I doubt that matters, compared to it being at least somewhat self-describing and nerd-readable.
That, of course, won't work for the majority of the clone-mode drivers which *have* to store a binary image of the radio to do their work. However, for a driver that is faking clone-mode behavior with a live-mode driver, storing the things we want symbolically would also be an option. But, we have the same concerns over testing rigor to make sure we don't break our own format, and we have to make sure that things are forwards and backwards compatible across chirp versions. Storing exactly what the radio stores is a good way to basically put that out of our own control.
--Dan
Hi Dan,
Having been around file formats and their implementations for decades, I very much agree that creating a new format is something to avoid. It is exceptionally rare for the first version of a file format to be the be-all and end-all. A file format needs to be very carefully designed for extensibility, backwards compatibility, and future-proofing. These are not easy to do. I'd add that selecting a non-binary file format *representation* (e.g. XML, JSON) does not, by itself, solve these issues, because the file format still needs to be designed on top of that representation, with almost all of the same issues as a binary format. The representation may provide a language for expressing a file format, but it is not in itself a file format.
You may recall that I commented on the live mode versus clone mode approach for this driver back in October:
http://intrepid.danplanet.com/pipermail/chirp_devel/2020-October/006088.html http://intrepid.danplanet.com/pipermail/chirp_devel/2020-October/006090.html
It isn't clear to me that there are compelling reasons to have a clone mode driver for the IC-7300 in this particular case, and some of the issues mentioned with live mode did not seem valid, as far as I could determine.
From a user perspective, I would think that sticking to one design mode for
one type of radio would make more sense than either having different radios that are really live mode radios behaving differently with different types of drivers, or presenting two different drivers for one radio to the user, who then has to understand which one they should use and why.
My 2 cents.
Martin. KD6YAM
On Tue, Mar 9, 2021 at 4:35 PM Dan Smith via chirp_devel < chirp_devel@intrepid.danplanet.com> wrote:
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 l eaving 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 wo rk 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 _______________________________________________ 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
OK, I will drop the clone-mode version and make it live-mode using the icomciv.py module. Then we can talk about how to save some settings configurations.
On 3/9/2021 4:34 PM, Dan Smith via chirp_devel wrote:
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 l eaving 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 wo rk 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 _______________________________________________ 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
participants (4)
-
Dan Smith -
Greg Troxel -
Martin Cooper -
Rick (AA0RD) DeWitt