Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect RT95/VOX #1028

Merged
merged 8 commits into from
May 4, 2024
Merged

Detect RT95/VOX #1028

merged 8 commits into from
May 4, 2024

Conversation

kk7ds
Copy link
Owner

@kk7ds kk7ds commented May 2, 2024

  • Remove future library dependency
  • Refactor detection for class isolation
  • rt95: Detect VOX variant from RT95

@kk7ds
Copy link
Owner Author

kk7ds commented May 2, 2024

@KC9HI here's a proposal for making the RT95vox detected from the RT95 driver.

I had to do a lot of refactoring of the download/identification routines in order to make this work, but I don't have any radio that uses this driver to test with. I wrote a little test to try to prove to myself that it works, but it definitely needs a good test. If it doesn't work, show me some debug log stuff and I'll try to figure it out (we can open a bug for this to attach files and such to if you want).

Basically the way this feature works is we split the identification part of the routine out into a separate helper called detect_from_serial(). That returns the proper class that represents the radio it finds, which we then instantiate and call download (sync_in()) like normal. But, since the radio has already been put into program mode, download just assumes that's already done and plows ahead. The difference for the early part is just that we don't yet have a whole radio to pass around, just a list of classes, so we have to make that code all work without a full radio object.

Also, while I was in there, I found some side signaling going on with a global (HAS_VOX) which is problematic because if someone has multiple radios, we'll confuse which one is the VOX-having one because of the global. So this also makes that a return argument instead, but unless I'm missing somewhere else that's used, I think that part of the change should be fine.

Lastly,. it looks like there are a couple other "vox" variants, so perhaps we should apply the same to those as well?

@KC9HI
Copy link
Contributor

KC9HI commented May 2, 2024

Great. I was thinking about looking into this today, but spend most of my time outside working and trying to stay cool. I expected it to be over my head, and apparently it would have been. I have a set of Retevis radios but I'm not sure if I have one of the newer ones with 'VOX'. I'm pretty sure I did the development by working with someone that had one. But Retevis many have sent me one after the fact so I will look around for it when I dig out the originals that I will use for testing your changes.

@kk7ds
Copy link
Owner Author

kk7ds commented May 2, 2024

Okay, well, it's really not that complicated, it just looks like it because of all the moving around of stuff. If you can test with any of the models that driver supports then my confidence will be reasonably high I think. Maybe we can scare up a VOX-having user on the list or something after that.

@KC9HI
Copy link
Contributor

KC9HI commented May 3, 2024

I located my original RT95 radio and a new RT95 VOX radio. The driver correctly detects both radios when downloading. Uploading back to the same radio was successful for both. Nice!

The issues are as follows...

The first issue is the driver forgets which model it was working on after an upload when programming an RT95 VOX. To replicate the issue...
1 set the model selection is set to RT95 and download from RT95 VOX radio
2 upload tab back to RT95 VOX radio (RT95 VOX is auto selected)
3 go to upload again and model selection has changed to H777 (first in the list) instead of RT95
This only happens with the RT95 VOX model. The RT95 remains selected after following this sequence with the RT95

Issue 2a and issue 2b
When (accidentally) uploading an RT95 image when an RT95 VOX is connected and when (accidentally) uploading an RT95 VOX image when an RT95 is connected results in error messages that no longer explain what the problems is or what steps should be taken to correct the error. Perhaps these messages you be updated/reworded?

RT95 image to RT95 VOX radio
RT95 VOX image to RT95 radio

@kk7ds
Copy link
Owner Author

kk7ds commented May 3, 2024

The first issue is the driver forgets which model it was working on after an upload when programming an RT95 VOX. To replicate the issue... 1 set the model selection is set to RT95 and download from RT95 VOX radio 2 upload tab back to RT95 VOX radio (RT95 VOX is auto selected) 3 go to upload again and model selection has changed to H777 (first in the list) instead of RT95 This only happens with the RT95 VOX model. The RT95 remains selected after following this sequence with the RT95

Hmm, okay I can't reproduce this with a UV-K5 and UV-K5(egzumer) which should be the same configuration, but let me ponder a bit.

Issue 2a and issue 2b When (accidentally) uploading an RT95 image when an RT95 VOX is connected and when (accidentally) uploading an RT95 VOX image when an RT95 is connected results in error messages that no longer explain what the problems is or what steps should be taken to correct the error. Perhaps these messages you be updated/reworded?

Probably just that this is happening in the early detection routine that is now separate from the download part, and a different error is bubbling up. I'll see if I can figure that out.

Works better than I was expecting for an untested shot in the dark, thanks!

@KC9HI
Copy link
Contributor

KC9HI commented May 3, 2024

The first error was added to appease Retevis. There is absolutely nothing on the new box or new radio to let anyone know it is any different than the original RT95. They could have at lease called it V2 or Gen2 or something. So when I developed the support I just tacked on the VOX because that was the major feature that was included.

New radio owners were selecting RT95 (that's what the box, radio and manual showed) to download from their radio and when it failed they were contacting Retevis. Retevis contacted me so I added the message to try RT95 VOX. That shouldn't happen any more for downloading. For uploading it just needs to say something like "Image does not match connected radio" or something similar.

@kk7ds
Copy link
Owner Author

kk7ds commented May 3, 2024

Okay, yeah, there's a logic error in the code that decides whether or not to report that message. Like I'm not sure how it worked before. So let me just simplify it all to just say "wrong model" or something (has to work grammatically for upload and download) and see if that removes the need to even care about vox-or-not in this code.

Still haven't figured out the second-upload model selection thing. Anything relevant in the log when the second attempt fails to select the right model?

@KC9HI
Copy link
Contributor

KC9HI commented May 3, 2024

I've never figured out where the log file is on my development computer. Here is the text captured from the console. The first part might give a clues as to why the model selection is falling back to H777. When downloading it still needs to indicate when the radio being downloaded is not in the 'allowed list' so it can be identified and added.

WARNING: Last vendor/model (Retevis/RT95 VOX) not found
ERROR: Failed to clone: Radio identified as model with VOX
Try Retevis-RT95 VOX
Traceback (most recent call last):
File "C:\Users\Jim\mychirpgit\chirp\chirp\wxui\clone.py", line 78, in run
self._fn()
File "C:\Users\Jim\mychirpgit\chirp\chirp\drivers\anytone778uv.py", line 722, in sync_out
do_upload(self)
File "C:\Users\Jim\mychirpgit\chirp\chirp\drivers\anytone778uv.py", line 521, in do_upload
bandlimit = get_bandlimit_from_ver(radio, ver_response)
File "C:\Users\Jim\mychirpgit\chirp\chirp\drivers\anytone778uv.py", line 423, in get_bandlimit_from_ver
raise errors.RadioError(
chirp.errors.RadioError: Radio identified as model with VOX
Try Retevis-RT95 VOX
WARNING: Stopping clone thread
ERROR: Failed to clone: Radio version not in allowed list for Retevis-RT95 VOX:
000: 49 52 54 39 35 00 00 00 IRT95...
008: 01 56 31 30 30 00 00 06 .V100...
Traceback (most recent call last):
File "C:\Users\Jim\mychirpgit\chirp\chirp\wxui\clone.py", line 78, in run
self._fn()
File "C:\Users\Jim\mychirpgit\chirp\chirp\drivers\anytone778uv.py", line 722, in sync_out
do_upload(self)
File "C:\Users\Jim\mychirpgit\chirp\chirp\drivers\anytone778uv.py", line 521, in do_upload
bandlimit = get_bandlimit_from_ver(radio, ver_response)
File "C:\Users\Jim\mychirpgit\chirp\chirp\drivers\anytone778uv.py", line 428, in get_bandlimit_from_ver
raise errors.RadioError(
chirp.errors.RadioError: Radio version not in allowed list for Retevis-RT95 VOX:
000: 49 52 54 39 35 00 00 00 IRT95...
008: 01 56 31 30 30 00 00 06 .V100...

@kk7ds
Copy link
Owner Author

kk7ds commented May 3, 2024

I'm looking for debug log content from the "first issue" above where the second upload attempt doesn't select the right model. Run chirp with -vvv and you'll get the debug log to the console. Or you can run with --log test.log to get it as a file. I assume you're not seeing a real debug.log generated in your profile dir because you're running development mode from the console.

I think I've got the wrong-model thing fixed locally, just need to resolve the "first issue" above.

@KC9HI
Copy link
Contributor

KC9HI commented May 3, 2024

Can I do this tomorrow? I've already put everything away for the night. I can get it back out and run through it again if I need too.

@kk7ds
Copy link
Owner Author

kk7ds commented May 3, 2024

Yep, of course, no worries. I'll push up the message change now just so it's there. Thanks!

@kk7ds
Copy link
Owner Author

kk7ds commented May 3, 2024

The first issue is the driver forgets which model it was working on after an upload when programming an RT95 VOX. To replicate the issue...
1 set the model selection is set to RT95 and download from RT95 VOX radio
2 upload tab back to RT95 VOX radio (RT95 VOX is auto selected)
3 go to upload again and model selection has changed to H777 (first in the list) instead of RT95 This only happens with the RT95 VOX model. The RT95 remains selected after following this sequence with the RT95

Aha, I think I got it. I think in item 3 above you mean "download". So: download, upload, download, the third case resets the model to the first one in the vendor list. If download, upload, upload works as expected, because upload forces the model in the box anyway.

I understand why this is happening, I just need to figure out how I want to fix it. Thanks!

@KC9HI
Copy link
Contributor

KC9HI commented May 3, 2024

Yes... exactly, I meant 'download'. Sorry.

@KC9HI
Copy link
Contributor

KC9HI commented May 3, 2024

I don't think you are needing it any longer, but just in case here is the console output with the -vvv switch of a download -> upload -> download cloning sequence from a Retevis RT95 with VOX.

console.txt

@kk7ds
Copy link
Owner Author

kk7ds commented May 3, 2024

Yeah, I think the latest code in here should fix the problem.. have you re-pulled today and still see it?

@KC9HI
Copy link
Contributor

KC9HI commented May 3, 2024

No. I didn't realize that it was ready to test. I will do it right now.

@KC9HI
Copy link
Contributor

KC9HI commented May 3, 2024

That appears to have it sorted out. Nice.

@kk7ds
Copy link
Owner Author

kk7ds commented May 3, 2024

Okay, should we do the same for AnyTone778UVvox and CRTMicronUVvox ? If so, do you have those for testing?

@KC9HI
Copy link
Contributor

KC9HI commented May 3, 2024

I believe that if it is possible, it should also be done for the AnyTone and CRT models. Unfortunately I do not have access to those radio variants.

The only thing that I might be able to do would be to temporarily change the ALLOWED_RADIO_TYPES so that my radios are detected as the AnyTone and CRT variants for testing.

@kk7ds
Copy link
Owner Author

kk7ds commented May 3, 2024

Okay, probably no point in doing that, since they're all just so similar. I'll just tack one on to the end of this here to make them detected. We can maybe ask on the users list after the next build, or just wait to see if someone complains :)

I also thought about making it so that if you select a detectable model, it would give you an option to choose between "just detect it for me" and a manual override for the sub-models, which could potentially offer a bypass if the detection doesn't work properly or something. Need to do some more thinking on that though.

kk7ds added 7 commits May 3, 2024 17:34
This makes us keep detected models separate per class that they
were @detected_by. Before this, we would end up with a mashup of all
the detected subclasses in the highest parent class that was a detect
target (i.e. TID H3 and H8 mixed together). Now, we keep them
separate at import/class define time so we don't have to further
filter them. Also, this builds in the include-self behavior to
detected_models() to avoid having to repeat that everwhere.

This also fixes a download..upload..download bug where we will corrupt
the last_model with the detected model name, which isn't selectable
on download (since it's detected and not listed).
Just like the other "vox" variants, detect these models from their
base as well.
@kk7ds kk7ds merged commit e95140f into master May 4, 2024
6 checks passed
@kk7ds kk7ds deleted the remove-future branch May 4, 2024 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants