[chirp_devel] Add new radio Icom IC-E90/T90/T90A

Dan Smith
Thu Sep 19 13:22:33 PDT 2019


> - implementing it as the special channels as you are suggesting:
> the problem is that the TV channels are different - they occupy
> just 8 bytes instead of the 16 bytes regular/special channels, there
> are just modulation AM/WFM, frequency (which is calculated differently),
> name (different name size of other channels). And there are
> no duplex, no offset, no tone, no tuning step and maybe no other
> things. I am currently not sure whether it's possible to hack it
> together with the "normal" special channels and not confuse user.

Yeah, but lots of radios with special channels have these limitations. It's a little unfortunate, but in line with other ones. FM broadcast channels, 60m fixed-frequency-and-mode HF channels, etc.

> - implementing it in the setting dialog: it would lost the memedit
> channels magic, e.g no copy&paste, reordering etc., I didn't want
> to reimplement the memedit.
> 
> - implementing it as subdevice with different feature list:
> I choose this variant because I thought it's the most easy way
> and in the end it really was easy.
> 
> But if your opinion is different, I can change it.

My opinion is that multi-device radio drivers tend to be a little buggy-er than others, and it's annoyingly hacky in the tests to make it work right. If you feel strongly, then that's fine, but especially since I expect nobody to have any real reason to use those channels anymore, it's probably better to opt for simplicity. Your call.

> I am rstriping(0x00) which is used by the radio as a pad for unfilled channels.
> The radio supports space in its charset, so I am not rstriping it. What confused
> me a bit is that the space was included in both 'expected'/'obtained' reports
> of the testsuite.

It's a CHIRP requirement that the driver not return a memory with trailing whitespace. It's annoying to edit that way, and tends to propagate through import and copy/paste operations.

> This is because this radio uses longer icf lines, but it is still small address,
> i.e. it doesn't use the 10 chars prefix and it uses the 6 chars prefix,
> but it's detected as the 10 chars prefix, because the detection routine wasn't
> flexible enough. This is an attempt which assumes that the length of the data line
> (after the prefix) will be multiply of 8 (I thought that sane people would do it
> this way :) , so the rest has to be the prefix. This calculation works for the
> 38 chars line length as was hardcoded before and also for the IC-E90. If there is
> a better way how to detect this, I am ready to change it.

Put a big fat comment over it, at the very least. Explain what it used to do and what your rationale is for doing it this way. Otherwise it'll be hard to suss out later without blame.

> To be honest I don't know what's the difference between T90 and T90A, I saw
> there are radios around which uses both labels on the case. I also noticed
> there are manuals for both. So I will probably do more investigation about
> it - i.e. I will probably diff the manuals. Or if by an accident is here
> somebody who knows more, please let me know. But if you think it's not
> necessary, I will drop it.

I think it's a marketing thing. The -A variants are usually for North America, -E is for Europe and without either means something else. Not all models have all those variants. In other cases where necessary, we support the A and the E, but I have never heard of an A not working for the un-suffixed variant.

> I think it should also work. I am using the self.special also in the
> get_memory/set_memory, but I assume both are called after the get_features call.
> So NP, I will change it.

If you use it there, don't put it in get_features() and expect it to have already been called. Either keep it in init() or make it an @property so it gets set the first time it's used or something. We instantiate drivers for various reasons all over the code and test suite, so doing unnecessary work in there, even if quick, is wasteful.

> Sorry, I missed this, NP, I will post git formatted patch later, probably during
> next week after I return from my vacation and probably after we resolve other
> problems with this patch.

Okay, thanks.

> Do you want the binary img file to be included
> in the git formatted patch? This was probably the only reason why I didn't
> send it originally this way :)

Mercurial can digest git binary patches, so this *should* work, but it's also easy to just attach it to the bug so I can wget it from the machine that digests the patches.

Thanks!

--Dan


More information about the chirp_devel mailing list