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

Add Radtel RT-493 Driver #1270

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

paulober
Copy link

rt-493: Added Driver

This PR adds a driver for the Radtel RT-493.
I implemented it as fake CloneModeRadio as it wants a request per two memory channels when reading but it's simpler to just dump all at once due to the layout of settings (the official app does it the same way).

I'm currently not sure what to put into the valid_bands because the European version is 430 MHz to 440 MHz but the device can do 400 MHz to 470 MHz.

Copy link
Owner

@kk7ds kk7ds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for working on this model. I didn't do a thorough review of this because I think there are a bunch of structural things that need to change in this driver to get it close. Probably in order:

  1. Download the entire memory of the radio as an image, upload specific sections if necessary (not common)
  2. Use bitwise for the native formats, don't do all the hand-encoding of things and forcing it raw into the data structures
  3. The communication timing stuff and the multiple wraps of read() and write() are anti-patterns that should be avoided.

data = bytearray()

_enter_programming_mode(radio)
for i in range(1, radio.MEM_ROWS, 2):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, as above, please do not try to download selections of the memory like this. Download the entire thing always.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by this?
There is no command to download everything at once (at least not one that I know of) therefore I have to send a read command per 2 memory channels. And I put it in a radio.MEM_ROWS so this could be overwriten by alias driver classes.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no command to "read a memory" or "read two memories" either. You're just reading memory in 0x20-byte chunks, you just happen to be starting at the address of the first memory. You should start at zero and read the full size of memory. This radio appears to behave just like 80% of all the other chinese radios that copied kenwood's model, so you can look at those for inspiration.

Here's an example that is probably almost exactly what you need to do: https://github.com/kk7ds/chirp/blob/master/chirp/drivers/radtel_rt490.py#L364 and TBH, you might even be able to just subclass that radio. In fact the memory structure of that radio looks quite similar to what you have here, so it might be best to re-use a lot more of that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it with 0x40 at the end (like rt490 by default) but that just brakes the communication.

@paulober
Copy link
Author

@kk7ds Thanks for the feedback i'll look into it. For now, how can i build a windows installer so people can test it?

@kk7ds
Copy link
Owner

kk7ds commented Feb 25, 2025

Just attach the radtel_rt493.py file to an issue and they can load it directly using this procedure: https://www.chirpmyradio.com/projects/chirp/wiki/LoadingTestModules ... no need to build a full binary install.

@paulober
Copy link
Author

You mentioned that maybe uploading only stuff that changed would be a good idea (if i remember correclty).
I just found out that if you upload for example only the channels and not the aditional memory rows coming afterwards with some settings you will lose their content. In the current version i fogot to update the limit in the uplaod function causing los of every memory line after the latest uploaded one.

@kk7ds
Copy link
Owner

kk7ds commented Feb 25, 2025

Not that it's a good idea, that it's the way to handle the case where you should only upload part of the memory. Some radios store calibration settings in the main area of memory which should NOT be uploaded from one radio to another. If that's the case here, you want to download everything, but only upload the regions that should be actually sent back to the radio. This is how OEM software works and is the case for many of the radio in-tree. Better-designed radios store calibration settings in a different section of memory and expect the whole image to be uploaded every time.

But yes, what you do not want to do is try to pick-and-choose what you download and upload from the radio unless you definitely know what you're doing because it's easy to get some settings not updated that are dependent on other changes you make (like memory content and scan flags stored separately). It is by far much better to download and upload the whole image of the radio unless you have a specific need not to, which is why I want you to (at least) download the whole thing.

@paulober
Copy link
Author

I just implemented most of the changes you requested / recommended. I must admit it now looks a lot cleaner. This was the first time writing a software for chirp and radios in general so i'm sorry if I might not polished everything right at the beginning.
The timing stuff you were confused about isn't actually necessarry. I just had some wired issues during protocol analysis so I thought I use the same delays as the OEM software. But now I know the correct order of the accept bytes was the issue and the timing actually is irelevant.

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