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@intrepid.danplanet.com> wrote:
elif setting == "keyunlocked":
LOG.debug("Setting %s = %s" % (setting, notint(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,notint(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 beset "
"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@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers