[chirp_devel] [PATCH] [ft4] allow validation of RO frequencies [#6615]
# HG changeset patch # User DanClemmensen DanClemmensen@gmail.com # Date 1553532256 25200 # Mon Mar 25 09:44:16 2019 -0700 # Node ID 034168b01eceb10301f04dd8596c03aa1267099f # Parent 936bffe5c76c85e7dd626c448b0e9031d274235c [ft4] allow validation of RO frequencies [#6615] Fixes: #6615 This change to the UI does not implement any RO changes. It permits a river to implement such a change.
diff -r 936bffe5c76c -r 034168b01ece chirp/ui/memedit.py --- a/chirp/ui/memedit.py Tue Mar 19 12:58:02 2019 -0700 +++ b/chirp/ui/memedit.py Mon Mar 25 09:44:16 2019 -0700 @@ -137,6 +137,9 @@ was_filled, prev = self.store.get(iter, self.col("_filled"), colnum)
def set_offset(offset): + old_dup = self.store.get(iter, self.col(_("Duplex")))[0] + if old_dup in ["off", "split"]: + return if offset > 0: dup = "+" elif offset == 0:
# HG changeset patch # User DanClemmensen DanClemmensen@gmail.com # Date 1553532256 25200 # Mon Mar 25 09:44:16 2019 -0700 # Node ID 034168b01eceb10301f04dd8596c03aa1267099f # Parent 936bffe5c76c85e7dd626c448b0e9031d274235c [ft4] allow validation of RO frequencies [#6615] Fixes: #6615 This change to the UI does not implement any RO changes. It permits a river to implement such a change.
diff -r 936bffe5c76c -r 034168b01ece chirp/ui/memedit.py --- a/chirp/ui/memedit.py Tue Mar 19 12:58:02 2019 -0700 +++ b/chirp/ui/memedit.py Mon Mar 25 09:44:16 2019 -0700 @@ -137,6 +137,9 @@ was_filled, prev = self.store.get(iter, self.col("_filled"), colnum)
def set_offset(offset):
old_dup = self.store.get(iter, self.col(_("Duplex")))[0]if old_dup in ["off", "split"]:return if offset > 0: dup = "+" elif offset == 0:
I'm confused about what you're trying to do here, and having basically no explanation in the commit message isn't helping. I've read the bug and the reason for re-opening it, but I'm not sure what behavior you're describing. If I use the UV-5R driver, I can set a memory to "off" and it stays that way.
Can you please describe what you're fixing and why? If I have to use your driver and test image to reproduce, please put steps in the bug or something.
--Dan
Sorry about my tersness. The last thing you need at this point is to be distracted from the py3 work.
I submitted the memedit.py patch as a standalone instead of part of my RO channel patch because I know you prefer simple standalone patches where possible, and for good reason.
The problem is that I added TX Frequency validation to the validate_memory method of my driver. After failing using other approaches, I went back to a March 14 e-mail on the devel list where Jim Unroe pointed me at gmrs_uv1.py. He extends the validate_memory method. I did the same for my new code. The problem is that that the mem structure passed to validate_memory is not the same as the one later passed to mem_set: Specifically, If the "duplex" field is "off" in the UI column, mem.duplex is set to "" when it reaches validate_memory. If it is "split" in the UI column, it gets changed in some other way that makes it different from what is passed to set_memory. I looked for and failed to find the spot where the value is changed back to "off" or "split".
Without this change, My modifed driver rejects an RO channel (i.e., one that has duplex "off") that has a frequency that is invalid for TX, and the UI "duplex" column is forced back to "". With this change, my driver works.
I have not yet looked at the py3 branch, but I will after I get this patch into the main branch.
How would you like me to provide the modified driver for your testing? Just as a patch in the usual manner? No new image is needed.
On Wed, Mar 27, 2019 at 6:53 AM Dan Smith via chirp_devel < chirp_devel@intrepid.danplanet.com> wrote:
# HG changeset patch # User DanClemmensen DanClemmensen@gmail.com # Date 1553532256 25200 # Mon Mar 25 09:44:16 2019 -0700 # Node ID 034168b01eceb10301f04dd8596c03aa1267099f # Parent 936bffe5c76c85e7dd626c448b0e9031d274235c [ft4] allow validation of RO frequencies [#6615] Fixes: #6615 This change to the UI does not implement any RO changes. It permits a river to implement such a change.
diff -r 936bffe5c76c -r 034168b01ece chirp/ui/memedit.py --- a/chirp/ui/memedit.py Tue Mar 19 12:58:02 2019 -0700 +++ b/chirp/ui/memedit.py Mon Mar 25 09:44:16 2019 -0700 @@ -137,6 +137,9 @@ was_filled, prev = self.store.get(iter, self.col("_filled"),
colnum)
def set_offset(offset):
old_dup = self.store.get(iter, self.col(_("Duplex")))[0]if old_dup in ["off", "split"]:return if offset > 0: dup = "+" elif offset == 0:I'm confused about what you're trying to do here, and having basically no explanation in the commit message isn't helping. I've read the bug and the reason for re-opening it, but I'm not sure what behavior you're describing. If I use the UV-5R driver, I can set a memory to "off" and it stays that way.
Can you please describe what you're fixing and why? If I have to use your driver and test image to reproduce, please put steps in the bug or something.
--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
The problem is that I added TX Frequency validation to the validate_memory method of my driver. After failing using other approaches, I went back to a March 14 e-mail on the devel list where Jim Unroe pointed me at gmrs_uv1.py. He extends the validate_memory method. I did the same for my new code. The problem is that that the mem structure passed to validate_memory is not the same as the one later passed to mem_set: Specifically, If the "duplex" field is "off" in the UI column, mem.duplex is set to "" when it reaches validate_memory. If it is "split" in the UI column, it gets changed in some other way that makes it different from what is passed to set_memory. I looked for and failed to find the spot where the value is changed back to "off" or "split".
Okay, I'm not sure what you're describing there, but perhaps we should try to figure that out instead of making the change you've got here, or validate that what you have is correct.
Without this change, My modifed driver rejects an RO channel (i.e., one that has duplex "off") that has a frequency that is invalid for TX, and the UI "duplex" column is forced back to "". With this change, my driver works.
Okay, just having skimmed your driver patch, I'm concerned that you're changing too much underneath a bunch of drivers without accounting for it. I'll try to look closer soon.
After our last exchange, I've been thinking about how much it's worth doing all of this, I'm a little concerned that we're going down a rabbit hole here that might not really come with much payoff. Taking another radio as an example here, the modern Icom HF radios all "support" the 60m band. By default they can only transmit on the actually-allowed frequencies that the FCC has designated, but they can receive the whole band of course. When the FCC up and changed a few of them recently, older radios were stuck, unable to use the new channels because the regulations had been baked in. People that owned those radios performed the MARS/CAP mod (and the MARS people did too of course) so they could transmit on the entire 60m band in order to access the new frequencies. There's no way to tell from the outside that those mods have been done. If the CHIRP driver for these radios were to enforce the rules of an unmodified radio, the users of a modified radio wouldn't be able to do what they want.
Point being, I'm just not sure how useful it is to try to make CHIRP account for the fine-grained differences between TX/RX and TX-only frequencies in a radio. We're really just trying to manipulate the contents of the memory in a generic way for a ton of models, not provide holistic radio experience for one specific one with every possible quirk accounted for.
Would you be satisfied with some sort of visual indication in the UI of "A stock specimen of this model may not be able to transmit on the frequency you just put in" ?
--Dan
A warning instead of an error message would be fine: I'm trying to help the user, not limit the user.
My proposed patch affects only the radios supported by the ft4.py driver. We can limit the effect on other radios by asking other driver developers to avoid being overly restrictive. In the case of ft4.py, I have a patch waiting that will support the other radios in the family. This includes the "Asian" versions. The Asian versions are unlocked, so the warning will not occur. If a user unlocks a Euro or US version, this in effect makes it an Asian version, so the user can simply declare that their radio is Asian and carry on. The ft4.py driver cannot currently determine the radio type: there does not appear to be a difference in the portion of the memory that the driver accesses or in the ID string.
My only concern with my approach is that it is causing you to waste time supporting my obsession with fully supporting my own radio. I realize that full support of any particular radio is at most a secondary mission for CHIRP: you say so right in the FAQ.
Probably the easiest way to support the "warning, not error" approach from the driver side would be to add a "level" parameter to the message returned from validate_memory. Not sure how to then deal with a "warning" on the UI side. If you wish I can research this while you carry on with real CHIRP work.
On Mon, Apr 1, 2019 at 7:14 AM Dan Smith via chirp_devel < chirp_devel@intrepid.danplanet.com> wrote:
The problem is that I added TX Frequency validation to the
validate_memory method of my driver. After failing using other approaches, I went back to a March 14 e-mail on the devel list where Jim Unroe pointed me at gmrs_uv1.py. He extends the validate_memory method. I did the same for my new code. The problem is that that the mem structure passed to validate_memory is not the same as the one later passed to mem_set: Specifically, If the "duplex" field is "off" in the UI column, mem.duplex is set to "" when it reaches validate_memory. If it is "split" in the UI column, it gets changed in some other way that makes it different from what is passed to set_memory. I looked for and failed to find the spot where the value is changed back to "off" or "split".
Okay, I'm not sure what you're describing there, but perhaps we should try to figure that out instead of making the change you've got here, or validate that what you have is correct.
Without this change, My modifed driver rejects an RO channel (i.e., one
that has duplex "off") that has a frequency that is invalid for TX, and the UI "duplex" column is forced back to "". With this change, my driver works.
Okay, just having skimmed your driver patch, I'm concerned that you're changing too much underneath a bunch of drivers without accounting for it. I'll try to look closer soon.
After our last exchange, I've been thinking about how much it's worth doing all of this, I'm a little concerned that we're going down a rabbit hole here that might not really come with much payoff. Taking another radio as an example here, the modern Icom HF radios all "support" the 60m band. By default they can only transmit on the actually-allowed frequencies that the FCC has designated, but they can receive the whole band of course. When the FCC up and changed a few of them recently, older radios were stuck, unable to use the new channels because the regulations had been baked in. People that owned those radios performed the MARS/CAP mod (and the MARS people did too of course) so they could transmit on the entire 60m band in order to access the new frequencies. There's no way to tell from the outside that those mods have been done. If the CHIRP driver for these radios were to enforce the rules of an unmodified radio, the users of a modified radio wouldn't be able to do what they want.
Point being, I'm just not sure how useful it is to try to make CHIRP account for the fine-grained differences between TX/RX and TX-only frequencies in a radio. We're really just trying to manipulate the contents of the memory in a generic way for a ton of models, not provide holistic radio experience for one specific one with every possible quirk accounted for.
Would you be satisfied with some sort of visual indication in the UI of "A stock specimen of this model may not be able to transmit on the frequency you just put in" ?
--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
Ahh! chirp_common.py defines "ValidationWarining" in addition to "ValidationError".
We will still need the patch to memdetail.py so that validate_memory gets the correct value of "duplex", but a driver that wants to warn the user can do so if necessary. Now to go check on what the UI does when it sees a "ValidationWarning" ...
On Mon, Apr 1, 2019 at 9:22 AM Dan Clemmensen danclemmensen@gmail.com wrote:
A warning instead of an error message would be fine: I'm trying to help the user, not limit the user.
My proposed patch affects only the radios supported by the ft4.py driver. We can limit the effect on other radios by asking other driver developers to avoid being overly restrictive. In the case of ft4.py, I have a patch waiting that will support the other radios in the family. This includes the "Asian" versions. The Asian versions are unlocked, so the warning will not occur. If a user unlocks a Euro or US version, this in effect makes it an Asian version, so the user can simply declare that their radio is Asian and carry on. The ft4.py driver cannot currently determine the radio type: there does not appear to be a difference in the portion of the memory that the driver accesses or in the ID string.
My only concern with my approach is that it is causing you to waste time supporting my obsession with fully supporting my own radio. I realize that full support of any particular radio is at most a secondary mission for CHIRP: you say so right in the FAQ.
Probably the easiest way to support the "warning, not error" approach from the driver side would be to add a "level" parameter to the message returned from validate_memory. Not sure how to then deal with a "warning" on the UI side. If you wish I can research this while you carry on with real CHIRP work.
On Mon, Apr 1, 2019 at 7:14 AM Dan Smith via chirp_devel < chirp_devel@intrepid.danplanet.com> wrote:
The problem is that I added TX Frequency validation to the
validate_memory method of my driver. After failing using other approaches, I went back to a March 14 e-mail on the devel list where Jim Unroe pointed me at gmrs_uv1.py. He extends the validate_memory method. I did the same for my new code. The problem is that that the mem structure passed to validate_memory is not the same as the one later passed to mem_set: Specifically, If the "duplex" field is "off" in the UI column, mem.duplex is set to "" when it reaches validate_memory. If it is "split" in the UI column, it gets changed in some other way that makes it different from what is passed to set_memory. I looked for and failed to find the spot where the value is changed back to "off" or "split".
Okay, I'm not sure what you're describing there, but perhaps we should try to figure that out instead of making the change you've got here, or validate that what you have is correct.
Without this change, My modifed driver rejects an RO channel (i.e., one
that has duplex "off") that has a frequency that is invalid for TX, and the UI "duplex" column is forced back to "". With this change, my driver works.
Okay, just having skimmed your driver patch, I'm concerned that you're changing too much underneath a bunch of drivers without accounting for it. I'll try to look closer soon.
After our last exchange, I've been thinking about how much it's worth doing all of this, I'm a little concerned that we're going down a rabbit hole here that might not really come with much payoff. Taking another radio as an example here, the modern Icom HF radios all "support" the 60m band. By default they can only transmit on the actually-allowed frequencies that the FCC has designated, but they can receive the whole band of course. When the FCC up and changed a few of them recently, older radios were stuck, unable to use the new channels because the regulations had been baked in. People that owned those radios performed the MARS/CAP mod (and the MARS people did too of course) so they could transmit on the entire 60m band in order to access the new frequencies. There's no way to tell from the outside that those mods have been done. If the CHIRP driver for these radios were to enforce the rules of an unmodified radio, the users of a modified radio wouldn't be able to do what they want.
Point being, I'm just not sure how useful it is to try to make CHIRP account for the fine-grained differences between TX/RX and TX-only frequencies in a radio. We're really just trying to manipulate the contents of the memory in a generic way for a ton of models, not provide holistic radio experience for one specific one with every possible quirk accounted for.
Would you be satisfied with some sort of visual indication in the UI of "A stock specimen of this model may not be able to transmit on the frequency you just put in" ?
--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
participants (3)
-
Dan Clemmensen -
Dan Smith -
DanClemmensen