Hi Dan,
Thanks for looking through the patch, and thanks for chirp. New patch and image attached, which I think covers all your points. I've run the test suite with tox, and get a clean sweep with no errors.
A few specific comments in-line below.
On 15 May 20, Dan Smith via chirp_devel said:
Wow, I totally forgot about this bit type. I should be using this more. However, I think you don't actually want to use this. This is for arrays of homogenous flag bits, where we're mapping memory number into a bitfield of presence bits or something. Since this appears to be an array of heterogenous flags, I think you'd be better doing something like this:
Excellent - I hadn't twigged you could that, much easier to read. Done.
I think utils.hexprint() is what you want. It'll print hex/ascii which I hope would be enough.
hexprint() will do nicely, I stole reprdata from my hacky development scripts. Done.
Hmm, it looks to me like you're reading the memory out of order, which is fine, but also constructing the image out of order as well. It's cool to try to optimize the read, which looks like future work, but we need to construct a whole and faithful image. That means the image should mirror what is in the radio. You can read the mapping first, but you should construct data to fill the image for the sections you don't read (according to what it looks like in the radio) and make sure to stash the mapping information you read early into the memory map in the right location based on where it is in the radio. Also, make sure you're reading everything else if there is a gap between the end of the memory and those flags. The OEM software has the advantage of knowing what the memory layout of the radio should look like, but since we don't, we should try to capture as much of it faithfully as we can.
Done, I think, just let me check what I should be doing. Previously I was reading in only the bits of memory I knew what to do with, and storing them out of order. Understand why that's not desirable, so now I'm reading the whole memory, settings and everything, and only modifying things I understand.
Is that what you intended?
Please use python3-compatible syntax in new code (should be "except exception as e".
Done.
Since this is a new driver, please use the newer format so that this has a hope of being python3 compatible. That means using MemoryMapBytes() here and setting NEEDS_COMPAT_SERIAL=False on your radio class. See the tk8180 driver for one example. (There's not really much way you could have known this, without following the archives here, sorry but thanks).
Done.
In the final version we commit, I'd like this just to mirror what some of the other drivers mention. Things you haven't tested, concerns you have, or just a statement that it's experimental. People can file a bug report for issues, which we can assign to you. I'd like to not have direct emails in these just because it reduces visibility for everyone else that there may be a problem.
Done, now says: "This is experimental support for the AnyTone 778UV. Please send in bug and enhancement requests!"
This really needs to be implemented in the final version. DTCS is too core and common to not include it.
Done.
Cheers, Joe