[chirp_devel] [PATCH] Fix logic error preventing auto repeater setting in some cases
# HG changeset patch # User Dan Smith dsmith@danplanet.com # Date 1363140169 25200 # Node ID 39add30a1ee5ea0c652f5ff4a310bcb1c0426eca # Parent 443ea98c0840de12f8ee149ccd8eecc78bb69a51 Fix logic error preventing auto repeater setting in some cases
If autorpt is enabled and you enter a frequency into a blank channel that is the same as was previously just entered, the new item defaulting code fools ed_freq() into thinking that the frequency field was unchanged, and thus that the auto repeater logic should not be applied. This enhances the logic that determines if the frequency was changed to avoid this problem.
Fixes #683
diff -r 443ea98c0840 -r 39add30a1ee5 chirpui/memedit.py --- a/chirpui/memedit.py Tue Mar 05 09:49:47 2013 -0800 +++ b/chirpui/memedit.py Tue Mar 12 19:02:49 2013 -0700 @@ -123,7 +123,7 @@
def ed_freq(self, _foo, path, new, colnum): iter = self.store.get_iter(path) - prev, = self.store.get(iter, colnum) + was_filled, prev = self.store.get(iter, self.col("_filled"), colnum)
def set_offset(path, offset): if offset > 0: @@ -154,7 +154,8 @@ if not self._features.has_nostep_tuning: set_ts(chirp_common.required_step(new))
- if new and self._config.get_bool("autorpt") and new != prev: + is_changed = new != prev if was_filled else True + if new and self._config.get_bool("autorpt") and is_changed: try: band = chirp_common.freq_to_band(new) set_offset(path, 0)
Hi,
+ is_changed = new != prev if was_filled else True + if new and self._config.get_bool("autorpt") and is_changed:
If you drop self._config.get_bool("autorpt"), this reads: + is_changed = new != prev if was_filled else True + if new and is_changed:
Which is really strange.
On Wed, Mar 13, 2013 at 1:03 PM, Dan Smith dsmith@danplanet.com wrote:
# HG changeset patch # User Dan Smith dsmith@danplanet.com # Date 1363140169 25200 # Node ID 39add30a1ee5ea0c652f5ff4a310bcb1c0426eca # Parent 443ea98c0840de12f8ee149ccd8eecc78bb69a51 Fix logic error preventing auto repeater setting in some cases
If autorpt is enabled and you enter a frequency into a blank channel that is the same as was previously just entered, the new item defaulting code fools ed_freq() into thinking that the frequency field was unchanged, and thus that the auto repeater logic should not be applied. This enhances the logic that determines if the frequency was changed to avoid this problem.
Fixes #683
diff -r 443ea98c0840 -r 39add30a1ee5 chirpui/memedit.py --- a/chirpui/memedit.py Tue Mar 05 09:49:47 2013 -0800 +++ b/chirpui/memedit.py Tue Mar 12 19:02:49 2013 -0700 @@ -123,7 +123,7 @@
def ed_freq(self, _foo, path, new, colnum): iter = self.store.get_iter(path)
prev, = self.store.get(iter, colnum)
was_filled, prev = self.store.get(iter, self.col("_filled"),colnum)
def set_offset(path, offset): if offset > 0:@@ -154,7 +154,8 @@ if not self._features.has_nostep_tuning: set_ts(chirp_common.required_step(new))
if new and self._config.get_bool("autorpt") and new != prev:
is_changed = new != prev if was_filled else Trueif new and self._config.get_bool("autorpt") and is_changed: try: band = chirp_common.freq_to_band(new) set_offset(path, 0)
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
Hi,
Regardless of the wording of the variable names leading to confusion, this works for me.
On Wed, Mar 13, 2013 at 1:18 PM, Sean Burford sburford@google.com wrote:
Hi,
is_changed = new != prev if was_filled else Trueif new and self._config.get_bool("autorpt") and is_changed:If you drop self._config.get_bool("autorpt"), this reads:
is_changed = new != prev if was_filled else Trueif new and is_changed:Which is really strange.
On Wed, Mar 13, 2013 at 1:03 PM, Dan Smith dsmith@danplanet.com wrote:
# HG changeset patch # User Dan Smith dsmith@danplanet.com # Date 1363140169 25200 # Node ID 39add30a1ee5ea0c652f5ff4a310bcb1c0426eca # Parent 443ea98c0840de12f8ee149ccd8eecc78bb69a51 Fix logic error preventing auto repeater setting in some cases
If autorpt is enabled and you enter a frequency into a blank channel that is the same as was previously just entered, the new item defaulting code fools ed_freq() into thinking that the frequency field was unchanged, and thus that the auto repeater logic should not be applied. This enhances the logic that determines if the frequency was changed to avoid this problem.
Fixes #683
diff -r 443ea98c0840 -r 39add30a1ee5 chirpui/memedit.py --- a/chirpui/memedit.py Tue Mar 05 09:49:47 2013 -0800 +++ b/chirpui/memedit.py Tue Mar 12 19:02:49 2013 -0700 @@ -123,7 +123,7 @@
def ed_freq(self, _foo, path, new, colnum): iter = self.store.get_iter(path)
prev, = self.store.get(iter, colnum)
was_filled, prev = self.store.get(iter, self.col("_filled"),colnum)
def set_offset(path, offset): if offset > 0:@@ -154,7 +154,8 @@ if not self._features.has_nostep_tuning: set_ts(chirp_common.required_step(new))
if new and self._config.get_bool("autorpt") and new != prev:
is_changed = new != prev if was_filled else Trueif new and self._config.get_bool("autorpt") and is_changed: try: band = chirp_common.freq_to_band(new) set_offset(path, 0)
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
-- Sean Burford sburford@google.com
If you drop self._config.get_bool("autorpt"), this reads:
I'm not sure why dropping it has any relevance, but perhaps you're just saying it reads oddly?
is_changed = new != prev if was_filled else Trueif new and is_changed:
How about we change it to:
is_changed = new != prev if was_filled else True autorpt_enabled = self._config.get_bool("autorpt")
if new is not None and is_changed and autorpt_enabled: ....
?
In reality, that's why new is in the comparison (since it could be set to None right above in the parse_freq() bit). Perhaps the more explicit "is not None" reads better?
Hi,
On Wed, Mar 13, 2013 at 2:33 PM, Dan Smith dsmith@danplanet.com wrote:
If you drop self._config.get_bool("autorpt"), this reads:
I'm not sure why dropping it has any relevance, but perhaps you're just saying it reads oddly?
Sorry, yes, I meant that "new and changed" reads oddly.
is_changed = new != prev if was_filled else Trueif new and is_changed:How about we change it to:
is_changed = new != prev if was_filled else True autorpt_enabled = self._config.get_bool("autorpt")
if new is not None and is_changed and autorpt_enabled: ....
That looks good. Understanding that new is actually new_value makes the whole argument moot.
participants (2)
-
Dan Smith -
Sean Burford