OK cool. In that case, I'll leave the bug fix as is and make that the first patch. The goal
of the first patch is simply to ensure that if you download from a VX-8R, you don't 
generate trace back errors and you don't get a Settings menu at all. In other words,
ensure that the correct code gets called for a given variant. 

On an administrative note, should we just reject Issue 4881 and have me create a new
issue with the scope of work limited to the bug fix?

-Keith
KF7DRV

On Mon, Jun 5, 2017 at 1:29 PM, Dan Smith via chirp_devel <chirp_devel@intrepid.danplanet.com> wrote:
> Perhaps instead of leading with the refactor of the VX8DRadio class
> functions up to the VX8Radio class, I should first fix the obvious
> bug of the VXDRadio functions getting called for a VX-8R. Next, I
> would propose doing a patch that accommodates the memory layout
> differences between the VX-8R and VX-8DR/VX-8GE. Then I would do a
> patch that moves the common functions up to the base class (but
> don't yet get called for the VX-8R). The last patch would be to
> enable settings for the VX-8R by setting "has_settings" to True in
> the get_features function and adding the special version of the
> get_aprs_tx setting function in the base class (which would be
> overloaded by the get_aprs_tx function in the VX8DRadio class).
>
> Would this work OK for you?

Sounds awesome. I like awesome.

> Assuming I would start by fixing the  issue of the bug where the
> VX8DRadio functions are getting called when you download from a
> VX-8R, I am not really happy with my current "fix" anyway. I ended
> up adding separate models (VX-8R, VX-8DR, and VX-8GE) to the Yaesu
> pull-down menu. This works fine but long-time VX-8* users of Chirp
> would potentially not notice this change and select VX-8R when they
> have a VX-8DR. Additionally, the pull down menu is busier since it's
> now contaminated with variants.

I don't really think that's necessarily a bad thing. It's nice to be
able to do everything automatically all the time, but Yaesus aren't very
auto-detect-friendly. I don't really think it's a problem to make people
choose the specific model, nor do I really think we'll see flak from
users about it. Especially if we can point to "things work that didn't
work before" type issues.

> This problem doesn't occur when opening any of the VX-8R, VX-8DR, or
> VX-8GE image files. The specific model is apparently correctly
> determined by the ident string at the beginning of the image file.
> However, it definitely occurs when you download from the radio. I
> haven't yet ferreted out the difference between model detection via
> an image file open and model detection in the context of download
> from a radio. I've noticed that in other drivers there are explicit
> functions for identifying the radio. Is this some missing but
> required functionality not in the current VX8 driver?

Well, those detection routines are to probe for sub-models to do
different things during clone. I'm not sure that's really necessary
here, unless you want to unify all of the variants into a single class,
and just check properties of the image every time you want to do
something that differs between them. That would be okay if you think
it's worth it.

I assume that this is doable at all with Yaesus because they barf their
model number as the first thing when you hit clone on the radio right?
They're not (AFAIK) interrogate-able for model info like every other
radio on the planet.

> It also seems that Chirp has an Alias construct that seems to have
> replaced the Variant construct. I'm clearly very confused around this
> topic and a few pointers/suggestions would be most helpful!

Well, the reason we added that is because of all the models coming out
of China that are just re-badged with no other apparent differences.
Variant was added so you could have slight differences between the
European and North American model of a radio. However, if the same radio
is called "Foobar 9000" and "Whizbang 84" the variant isn't enough to
distinguish them (that, and you don't actually need a different subclass
for differences since there are none).