<div dir="ltr">Hi Dan,<div><br></div><div>Understand and I am willing. </div><div><br></div><div>Perhaps instead of leading with the refactor of the VX8DRadio class functions up to the</div><div>VX8Radio class, I should first fix the obvious bug of the VXDRadio functions getting called for</div><div>a VX-8R. Next, I would propose doing a patch that accommodates the memory layout differences</div><div>between the VX-8R and VX-8DR/VX-8GE. Then I would do a patch that moves the common functions</div><div>up to the base class (but don't yet get called for the VX-8R). The last patch would be to enable settings</div><div>for the VX-8R by setting "has_settings" to True in the get_features function and adding the special version</div><div>of the get_aprs_tx setting function in the base class (which would be overloaded by the get_aprs_tx function</div><div>in the VX8DRadio class). </div><div><br></div><div>Would this work OK for you? </div><div><br></div><div>Assuming I would start by fixing the issue of the bug where the VX8DRadio functions are getting</div><div>called when you download from a VX-8R, I am not really happy with my current "fix" anyway. </div><div>I ended up adding separate models (VX-8R, VX-8DR, and VX-8GE) to the Yaesu pull-down menu. </div><div>This works fine but long-time VX-8* users of Chirp would potentially not notice this change and select </div><div>VX-8R when they have a VX-8DR. Additionally, the pull down menu is busier since it's now</div><div>contaminated with variants. This problem doesn't occur when opening any of the VX-8R, VX-8DR, or VX-8GE</div><div>image files. The specific model is apparently correctly determined by the ident string at the beginning of the image file. </div><div>However, it definitely occurs when you download from the radio. I haven't yet ferreted out the difference </div><div>between model detection via an image file open and model detection in the context of download from a radio. </div><div>I've noticed that in other drivers there are explicit functions for identifying the radio. Is this some missing but </div><div>required functionality not in the current VX8 driver? It also seems that Chirp has an Alias construct that seems to have</div><div>replaced the Variant construct. I'm clearly very confused around this topic and a few pointers/suggestions</div><div>would be most helpful!</div><div><br></div><div>Cheers,</div><div><br></div><div>Keith</div><div>KF7DRV</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 5, 2017 at 11:36 AM, Dan Smith via chirp_devel <span dir="ltr"><<a href="mailto:chirp_devel@intrepid.danplanet.com" target="_blank">chirp_devel@intrepid.danplanet.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> The patch is somewhat lengthy due to the fact that I promoted most of<br>
> the VX8DRadio class functions to the VX8Radio base class. This left the<br>
> VX8DRadio class with just functions that were<br>
> somewhat different for the VX-8DR and VX-8GE (e.g.<br>
> _get_aprs_tx_settings) or unique to the VX-8DR and VX-8GE (e.g.<br>
> _get_aprs_smartbeacon).<br>
><br>
> Hopefully, I haven't introduced any PEP-8 violations..PyCharm is pretty<br>
> good at yelling at me about those. Please review and feel free to toss<br>
> the whole thing if it's felt that I'm off base<br>
> on this approach.<br>
<br>
</span>I'm okay with the refactor (in fact, I'm sure it's sorely needed), but<br>
do you think you could split this up into smaller pieces? Ideally, you'd<br>
lead with a refactor of things up into the base class, and then follow<br>
on with the "new" stuff in a separate patch. If we merge and release<br>
this, and then in six months someone reports a weird regression, it'll<br>
be hard to do anything other than revert this entire patch to see if it<br>
solves the problem.<br>
<br>
I know it'll be some work, but... are you willing?<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
--Dan<br>
</font></span><div class="HOEnZb"><div class="h5">______________________________<wbr>_________________<br>
chirp_devel mailing list<br>
<a href="mailto:chirp_devel@intrepid.danplanet.com">chirp_devel@intrepid.<wbr>danplanet.com</a><br>
<a href="http://intrepid.danplanet.com/mailman/listinfo/chirp_devel" rel="noreferrer" target="_blank">http://intrepid.danplanet.com/<wbr>mailman/listinfo/chirp_devel</a><br>
Developer docs: <a href="http://chirp.danplanet.com/projects/chirp/wiki/Developers" rel="noreferrer" target="_blank">http://chirp.danplanet.com/<wbr>projects/chirp/wiki/Developers</a><br>
</div></div></blockquote></div><br></div>