[chirp_devel] [PATCH 2 of 3] [tg_uv2p] (revised) Inhibit keypad lock when in dual watch or crossmode. Fixes forth issue in #9939

[Ran Katz]

Hi Dan,
Thanks again,

I will submit a patch shortly (running tests now) with the added "change"
logic you proposed.
I also removed the explicit "good" setting from both settings - as the
radio is not really broken - it is functional but is locked in the not
normal) mode you are in and cannot be unlocked from the keypad - only from
chirp, so if the user has such a setting , maybe  it is intentional?
furthermore it will keep the UI sync'd with the actual setting.

Is there a way to raise an "informative" message ? (not an exception that
will abort the set_settings loop) so that the user can be notified he has a
"special" configuration?

Ran


On Sun, Aug 21, 2022 at 6:13 PM Dan Smith via chirp_devel <
chirp_devel at intrepid.danplanet.com> wrote:

> >                     elif setting == "keyunlocked":
> > -                        LOG.debug("Setting %s = %s" % (setting, not
> int(element.value)))
> > +                        if int(element.value):
> > +                            if (getattr(obj, "rxmode") != 0x02):
> > +                                LOG.debug("Setting %s = %s" % (setting,
> True))
> > +                                setattr(obj, setting, True)
> > +                                raise errors.InvalidValueError(
> > +                                    "Keypad lock not allowed in "
> > +                                    "Dual-Watch or CrossMode")
> > +                        LOG.debug("Setting %s = %s" % (setting,
> > +                                                       not
> int(element.value)))
> >                         setattr(obj, setting, not int(element.value))
>
> Since you're always raising, I think this is likely to be confusing
> because it's going to raise an exception to people who haven't changed that
> setting, and abort making the actual change they want, right?
>
> Meaning, let's say I have a radio with the settings already set to the
> incompatible state (i.e. in dual-watch and keylock enabled). I open up the
> settings to make a change to something completely different like the
> backlight timer or something. When I do that, the UI is going to try to set
> all the settings, including the two incompatible ones that I didn't touch.
> I'll get an error and then potentially my backlight setting won't be
> changed if it is later in the list than the keylock one above (since the
> exception will abort our loop).
>
> So I think you need some additional logic to check both stored values
> before you raise to avoid this breakage, Something like:
>
> if setting_keylock:
>     if is_dualwatch and now is_keylocked and setting_keylock.value ==
> locked:
>         raise Error
>
> So basically, only raise the exception if they're changing keylock (or
> dual watch) to a bad combination. It looks like you're also asserting the
> keylock off before you raise, which would fix an existing broken radio,
> right? That probably makes sense to include in there regardless, but I
> think you should remove the explicit setting from this section:
>
>
> > +                    elif setting == "rxmode":
> > +                        if int(element.value) != 2:
> > +                            if not (getattr(obj, "keyunlocked")):
> > +                                LOG.debug("Setting %s = %s" % (setting,
> > +
>  "Normal"))
> > +                                setattr(obj, setting, 0x02)
> > +                                raise errors.InvalidValueError(
> > +                                    "Dual-Watch or CrossMode can not be
> set "
> > +                                    "when keypad is locked")
> > +                        LOG.debug("Setting %s = %s" % (setting,
> element.value))
> > +                        setattr(obj, setting, element.value)
>
> I was going to say that this will "fight" the other one and end up with
> both values changed in a radio with those flags conflicting in memory.
> However, it won't - only the first one to be encountered will do that, but
> which one will get flipped is not deterministic. I think flipping off the
> keylock is the better "fix the problem for them" approach.
>
> --Dan
>
> _______________________________________________
> chirp_devel mailing list
> chirp_devel at intrepid.danplanet.com
> http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
> Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20220821/7ddcdf85/attachment-0001.html