Hi Dan, thanks for the feedback. I was also surprised at the differences (especially for the first time I've done this!), so like you said, hopefully this is a one-time pretty standard change going forward.

I've refactored the changes into two patches: one that adds raw support for icf.py, and one that adds the driver. I also got my hg setup to do the patchbomb, so I'll send the patches that way to the mailing list in just a minute. Note I also found a bug in my code doing this, so, yay! Perhaps not too surprisingly, in this new raw mode, the radio escapes control bytes sent to the computer just as we're expected to escape control bytes sent to the radio.

Cheers,

Rhett

On Sun, Jan 14, 2018 at 11:36 AM, Dan Smith via chirp_devel <chirp_devel@intrepid.danplanet.com> wrote:

Hi Rhett,

Sorry this took so long to get to.

Note that if you can inline the patches (like the patchbomb extension for hg will do for you) it's a lot easier for me to review them. It's a minor thing, but...it helps :)

> I've attached a patch file for the Icom IC-2730A (fixes #2745). There is also an image attached to the bug for tests.
>
> Please review and send feedback, especially on any suggested changes to how I've modified icf.py to support this radio, as it differs from previous Icom radios.

I'm really surprised to see some of the changes to the icf driver. One of the nicest thing about icom radios is that they all use the same (albeit kinda silly) protocol. Hopefully they're just switching to what you have done and we won't see a wide divergence going forward.

Would it be possible to refactor the clone mode driver into a base one that implements the original protocol and then a new subclass "raw" one? That might mean changing some of the module-level things to instance methods so you can override them in the subclass. I think that would be cleaner than passing the flags around. It would also be nice to have that cleanup/refactor as a separate patch from your driver add. Again, the patchbomb extension makes it easy to send a stack of patches. Info here:

https://chirp.danplanet.com/projects/chirp/wiki/DevelopersProcess#Sending-a-change

That said, the driver add looks reasonable.

Let me know what you think about the refactor and then we can go forward (and I'll try not to be so tardy in handling it).

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