[chirp_devel] IC-2730 Update; 2nd try
OK, here is the modified JC-2730 driver update. Back to Unix EOL chars, and many style fixes. I tried to use function calls in get_settings to make it more readable but I think it only made it worse, so I reverted back to the original all-in-line format. I have to respectfully disagree on the Information prompt; the manuals either do not mention these issues, or are very confusing. Issue #2745, items #54 and #55 both attest to this. I still can't get the cpep8.py test to run under windows... import errors.
OK, here is the modified JC-2730 driver update. Back to Unix EOL chars, and many style fixes.
Yep, this applies, and passes the style checks, thanks.
I have to respectfully disagree on the Information prompt; the manuals either do not mention these issues, or are very confusing. Issue #2745, items #54 and #55 both attest to this.
The text of the info prompt should relate to usage via CHIRP or something visible in the programming process. People mistakenly file issues against CHIRP for all manner of radio bugs, operating system issues, driver troubles, etc. That doesn't mean they're something we need to handle :)
('NOTE 1: Click the Special Channels tab on the main screen to ''view the C0 and C1 frequencies as channels 1000 and 1001.\n'
I didn't look at this closely enough on my first pass, but this is the wrong way to handle the special channels, and thus this info item won't be needed. Check out some of the other drivers that implement these, like ft817.py -- it has a ton of them. In summary, if you declare them as special, you can give them "extended names" like "C0" and make them sort at the top of the list, when enabled, as the radio does. Making them look like the 1000th and 1001st memory isn't the proper way.
'NOTE 2: CI-V is the control inteface protocol used to both'' clone and control the rig. The factory-standard CI-V address'' for the IC-2730a is hex 90.\n''NOTE 3: If there is no audio from the B-side, check that the ''programming interface cable is removed from speaker jack 2.\n'
These have nothing to do with CHIRP's usage of the radio, so they don't belong here. They belong on some wiki somewhere.
'NOTE 4: Enabling Weather Alert will cause an interupt every ''5 seconds when the band is recieving non-weather signals.\n'
At a minimum "receiving" should be spelled properly, but again, this is not specific to CHIRP. This is something that should be in the manual. If it's not, that's unfortunate, but it's not related to CHIRP. I can almost guarantee you that if you put this here, someone will open a bug and say "CHIRP should choose every 10 seconds not every 5", thinking that this driver is making that call about how often to check.
'Note 5: Bank names can only be editted in the radio menu.\n'
This changes format from NOTE to Note. "Edited" is misspelled. Further, I don't know what this means. Are you saying "CHIRP doesn't support editing the bank names so you have to do this on the radio" ? If so, I strongly suggest that we can eliminate this, as enumerating all the features the driver *doesn't* support would be silly.
'Note 6: CLONE ERROR on upload is usaually the result of an ''invalid frequency.\n'
This means the driver screwed up. Icom radio are extremely pedantic (in a good way) about the memory image and they will refuse an upload the instant you send it something invalid. However, that means the driver formatted something incorrectly or allowed the user to put something into memory that the radio won't tolerate. So, you can leave this in here if you want, but it needs to say that such a situation is a *bug*, should be reported, and is not the fault of the user.
So, of everything here, I'm willing to put "Note 5" in, with different wording, but the rest of the stuff doesn't belong. If you look at the three other drivers that put content here, it's specifically to call out driver/implementation quirks.
Otherwise, at quick glance, the functional changes look good and the style/patch formatting is good now, thanks!
--Dan
OK, you convinced me. I'll re-submit tomorrow without the info prompt. I think you are right that it will only cause more grief. I will also move C0 and C1 to the top of the list as negative channels, as I did with the FT-450D and TYT TH-UV8000 (which will also be submitted tomorrow once I fix some indentation issues).
Rick DeWitt AA0RD Sequim, Washington, USA 360-681-3494
On 6/27/2019 3:28 PM, Dan Smith via chirp_devel wrote:
OK, here is the modified JC-2730 driver update. Back to Unix EOL chars, and many style fixes.
Yep, this applies, and passes the style checks, thanks.
I have to respectfully disagree on the Information prompt; the manuals either do not mention these issues, or are very confusing. Issue #2745, items #54 and #55 both attest to this.
The text of the info prompt should relate to usage via CHIRP or something visible in the programming process. People mistakenly file issues against CHIRP for all manner of radio bugs, operating system issues, driver troubles, etc. That doesn't mean they're something we need to handle :)
('NOTE 1: Click the Special Channels tab on the main screen to ''view the C0 and C1 frequencies as channels 1000 and 1001.\n'I didn't look at this closely enough on my first pass, but this is the wrong way to handle the special channels, and thus this info item won't be needed. Check out some of the other drivers that implement these, like ft817.py -- it has a ton of them. In summary, if you declare them as special, you can give them "extended names" like "C0" and make them sort at the top of the list, when enabled, as the radio does. Making them look like the 1000th and 1001st memory isn't the proper way.
'NOTE 2: CI-V is the control inteface protocol used to both'' clone and control the rig. The factory-standard CI-V address'' for the IC-2730a is hex 90.\n''NOTE 3: If there is no audio from the B-side, check that the ''programming interface cable is removed from speaker jack 2.\n'These have nothing to do with CHIRP's usage of the radio, so they don't belong here. They belong on some wiki somewhere.
'NOTE 4: Enabling Weather Alert will cause an interupt every ''5 seconds when the band is recieving non-weather signals.\n'At a minimum "receiving" should be spelled properly, but again, this is not specific to CHIRP. This is something that should be in the manual. If it's not, that's unfortunate, but it's not related to CHIRP. I can almost guarantee you that if you put this here, someone will open a bug and say "CHIRP should choose every 10 seconds not every 5", thinking that this driver is making that call about how often to check.
'Note 5: Bank names can only be editted in the radio menu.\n'This changes format from NOTE to Note. "Edited" is misspelled. Further, I don't know what this means. Are you saying "CHIRP doesn't support editing the bank names so you have to do this on the radio" ? If so, I strongly suggest that we can eliminate this, as enumerating all the features the driver *doesn't* support would be silly.
'Note 6: CLONE ERROR on upload is usaually the result of an ''invalid frequency.\n'This means the driver screwed up. Icom radio are extremely pedantic (in a good way) about the memory image and they will refuse an upload the instant you send it something invalid. However, that means the driver formatted something incorrectly or allowed the user to put something into memory that the radio won't tolerate. So, you can leave this in here if you want, but it needs to say that such a situation is a *bug*, should be reported, and is not the fault of the user.
So, of everything here, I'm willing to put "Note 5" in, with different wording, but the rest of the stuff doesn't belong. If you look at the three other drivers that put content here, it's specifically to call out driver/implementation quirks.
Otherwise, at quick glance, the functional changes look good and the style/patch formatting is good now, thanks!
--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
OK, you convinced me. I'll re-submit tomorrow without the info prompt. I think you are right that it will only cause more grief. I will also move C0 and C1 to the top of the list as negative channels, as I did with the FT-450D and TYT TH-UV8000 (which will also be submitted tomorrow once I fix some indentation issues).
Sweet, thanks!
--Dan
participants (3)
-
Dan Smith -
Rick DeWitt -
Rick DeWitt AA0RD