[chirp_devel] [PATCH 1 of 2] [TH-350] Declare valid tuning steps for TYT TH-350
Uploads using this driver have always been slow. A patch submitted to address issue #3993 made matters worse.
Testing has indicated that the "sleep" that has been in use from the begining and was increased in issue 3993 is only necessary during downloading.
This patch addresses the issue by only having the "sleep(0.002)" effective during the download cloning process. Uploads with the "sleep" removed have been tested to be just as fast as downloads.
I've always been suspicious of the timing issues in this driver. I haven't had a radio to try to address them myself, nor do I really want to. The change you're making here doesn't make much sense to me because I would expect an upload, with a larger block of data, to be more of a problem for the slow radio than just sending the block request on the download side. But, since you've tested this and I can't, I merged it and I guess we'll see what, if any, response there is from the larger community and go from there.
I'm curious though: did anyone try just putting a delay in between the higher-level messages on the download side? Like, maybe it's not so much about inter-byte timing, but turnaround time? If the radio emits a big block of data, and then takes a moment to recover from that and be ready for the next command or ack, maybe we're sending that back too quickly? Meaning, if that's the case, maybe the following algorithm would be better as:
1. Request block 2. Read block 3. Wait 4. Request next block
As opposed to making the "Request next block" speak slowly. If the radio can consume a full block with no delays on upload, I just can't understand why the radio wouldn't be able to handle "please send block 2".
It also looks like from the bug maybe that was happening on block zero. Maybe we're doing the ident/prog mode part and then too quickly requesting the first block?
Anyway, I don't necessarily want to dredge it all up again, I just hate fiddling with tiny timing things like this, as it can really paper over the real case.
That said, thanks for continuing to care and for making this tweak. Hopefully it won't cause other problems :)
--Dan
On Fri, Apr 1, 2022 at 7:00 PM Dan Smith via chirp_devel chirp_devel@intrepid.danplanet.com wrote:
Uploads using this driver have always been slow. A patch submitted to address issue #3993 made matters worse.
Testing has indicated that the "sleep" that has been in use from the begining and was increased in issue 3993 is only necessary during downloading.
This patch addresses the issue by only having the "sleep(0.002)" effective during the download cloning process. Uploads with the "sleep" removed have been tested to be just as fast as downloads.
I've always been suspicious of the timing issues in this driver. I haven't had a radio to try to address them myself, nor do I really want to. The change you're making here doesn't make much sense to me because I would expect an upload, with a larger block of data, to be more of a problem for the slow radio than just sending the block request on the download side. But, since you've tested this and I can't, I merged it and I guess we'll see what, if any, response there is from the larger community and go from there.
I'm curious though: did anyone try just putting a delay in between the higher-level messages on the download side? Like, maybe it's not so much about inter-byte timing, but turnaround time? If the radio emits a big block of data, and then takes a moment to recover from that and be ready for the next command or ack, maybe we're sending that back too quickly? Meaning, if that's the case, maybe the following algorithm would be better as:
- Request block
- Read block
- Wait
- Request next block
As opposed to making the "Request next block" speak slowly. If the radio can consume a full block with no delays on upload, I just can't understand why the radio wouldn't be able to handle "please send block 2".
It also looks like from the bug maybe that was happening on block zero. Maybe we're doing the ident/prog mode part and then too quickly requesting the first block?
Anyway, I don't necessarily want to dredge it all up again, I just hate fiddling with tiny timing things like this, as it can really paper over the real case.
I do. I would like to solve this. I think I understand what you are saying above and I think I am making some progress. Would you like to see a diff against the original driver, what you just merged or the complete driver to look at?
That said, thanks for continuing to care and for making this tweak. Hopefully it won't cause other problems :)
--Dan
Jim
I do. I would like to solve this. I think I understand what you are saying above and I think I am making some progress. Would you like to see a diff against the original driver, what you just merged or the complete driver to look at?
Diffs against the current tree are best, yeah. I did merge what you had and that'll go out tomorrow regardless, so no rush on anything.
I'm also just armchair monday-morning debugger'ing this, so I'm just trying to think up things to try :)
--Dan
On Fri, Apr 1, 2022 at 9:02 PM Dan Smith via chirp_devel chirp_devel@intrepid.danplanet.com wrote:
I do. I would like to solve this. I think I understand what you are saying above and I think I am making some progress. Would you like to see a diff against the original driver, what you just merged or the complete driver to look at?
Diffs against the current tree are best, yeah. I did merge what you had and that'll go out tomorrow regardless, so no rush on anything.
That is what I expected. Thanks. Unless something that I didn't expect happens, I think everyone will be happy for the increased upload speed. Before I made this change I would have to go do something else because I couldn't stand waiting and watching while the 5+ minute upload was progressing.
I'm also just armchair monday-morning debugger'ing this, so I'm just trying to think up things to try :)
I appreciate it. This has bugged me for a long time (ever since Pavel and I co-authored it 2015 or 2016). I wasn't able to make anything that could read the new mobile radios so I was happy to have Pavel help me.
I believe the attached patch is moving in the right direction. I remove almost all of the timeout changes. The sleep() is now in the only place where it is required. The data blocks are no longer sent one byte at a time. So is this what you are looking for?
I tried reading and writing both a BTECH UV-25X4 radio (2017) and a BTECH GMRS-50X1 (2019) radio with Windows 10 and Linux Mint 19.3.
Jim
I believe the attached patch is moving in the right direction. I remove almost all of the timeout changes. The sleep() is now in the only place where it is required. The data blocks are no longer sent one byte at a time. So is this what you are looking for?
I tried reading and writing both a BTECH UV-25X4 radio (2017) and a BTECH GMRS-50X1 (2019) radio with Windows 10 and Linux Mint 19.3.
Oh man, yeah that's way better right? If that works reliably that's much less complex and contains much less potential for timing related issues, IMHO. Should we try to solicit some people with these radios to load the modification as a module and see if it's stable for all of them?
The only thing I might comment on is the sleep(0.002) after the ident. That's really (really) short and I wonder if it should be just a little longer without causing trouble? Like 0.1 maybe (100ms) or something. I can't imagine the radio requires that kind of turnaround and the 2ms you have is only two character timing units at 9600 baud.
But yeah, awesome, tanks Jim!
--Dan
Oh man, yeah that's way better right? If that works reliably that's much less complex and contains much less potential for timing related issues, IMHO. Should we try to solicit some people with these radios to load the modification as a module and see if it's stable for all of them?
Right! And yes, testing would be good.
I have been working with Matthew to get a new MCU Version added to CHIRP for his QYT KT-8900R. He provided a sample driver that he had modified on his own to get his radio going. It happened to be a very old driver so it didn't have the #3993 change that really slowed things down. When I provided a driver for him to test, he mentioned that it worked but he noticed the slow down. This morning I attached another test driver using these proposed changes. It, btech_M3B064_fast.py, can be acquired from issue #9816.
I can do quite a bit of testing myself. I have the following radios available: BTech UV-2501, UV-2501+220, UV-5001, WACCOM MINI-8900, LUITON LT-588UV, BTech UV-25X2, UV-25X4, UV-50X2, GMRS-50X1, QYT KT-8R and an Anysecu WP-9900 that I am currently working on. I am also able to test with Windows, Linux and macOS (if I have to).
I have also had John LaMartina (the webmaster of the miklor.com website) assist with the testing of the previous batch that just got merged.
What would be your recommendation for expanding testing to a wider group of radio owners?
The only thing I might comment on is the sleep(0.002) after the ident. That's really (really) short and I wonder if it should be just a little longer without causing trouble? Like 0.1 maybe (100ms) or something. I can't imagine the radio requires that kind of turnaround and the 2ms you have is only two character timing units at 9600 baud.
I changed the sleep(0.002) to sleep(0.1) this morning. So far I have encountered no issues with any testing that I have done since making that change.
Jim
I can do quite a bit of testing myself. I have the following radios available: BTech UV-2501, UV-2501+220, UV-5001, WACCOM MINI-8900, LUITON LT-588UV, BTech UV-25X2, UV-25X4, UV-50X2, GMRS-50X1, QYT KT-8R and an Anysecu WP-9900 that I am currently working on. I am also able to test with Windows, Linux and macOS (if I have to).
I'm starting to gather that your shack (or garage or storage unit or living room) looks like mine :)
I have also had John LaMartina (the webmaster of the miklor.com website) assist with the testing of the previous batch that just got merged.
What would be your recommendation for expanding testing to a wider group of radio owners?
Well, I'll leave it up to you about whether or not it's necessary. If you've got a bunch of radios and a bunch of platforms, maybe that's enough. Plus, if the change makes sense (which it does) then that's another point in favor. You could always solicit the chirp_users list for people that have that radio and point them at a bug with a module attached and instructions. But, I can understand if you think that might be a can of worms. The only thing that you're not covering with your list above is various slow computers on every windows version, every USB chip in the world, varying cosmic rays from different parts of the planet, etc.
But really, I'll leave it to you to decide on confidence level.
I changed the sleep(0.002) to sleep(0.1) this morning. So far I have encountered no issues with any testing that I have done since making that change.
Cool, I think that's better.
As always, thanks Jim!
--Dan
participants (2)
-
Dan Smith -
Jim Unroe