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

Here are the two files needed to run a Yaesu FT-450. I also included … #792

Closed
wants to merge 2 commits into from

Conversation

WCurlew
Copy link
Contributor

@WCurlew WCurlew commented Oct 26, 2023

…the test img as requested.

CHIRP PR Checklist

The following must be true before PRs can be merged:

  • All tests must be passing.
  • Commits should be squashed into logical units.
  • Commits should be rebased (or simply rebase-able in the web UI) on current master. Do not put merge commits in a PR.
  • Commits in a single PR should be related.
  • Major new features or bug fixes should reference a CHIRP issue.
  • New drivers should be accompanied by a test image in tests/images (except for thin aliases where the driver is sufficiently tested already).
  • All files must be GPLv3 licensed or contain no license verbiage. No additional restrictions can be placed on the usage (i.e. such as noncommercial).

Please also follow these guidelines:

  • Keep cleanups in separate commits from functional changes.
  • Please write a reasonable commit message, especially if making some change that isn't totally obvious (such as adding a new model, adding a feature, etc).
  • Do not add new py2-compatibility code (No new uses of six, future, etc).
  • All new drivers should set NEEDS_COMPAT_SERIAL=False and use MemoryMapBytes.
  • New drivers and radio models will affect the Python3 test matrix. You should regenerate this file with tox -emakesupported and include it in your commit.

…the test img as requested.

Signed-off-by: William Curlew <WCURLEW@juno.com>
@kk7ds
Copy link
Owner

kk7ds commented Oct 26, 2023

Hi, as noted by the failed check, it's not okay to just duplicate a whole driver like this. It duplicates all the problems in that driver (of which there are many) and creates a lot of maintenance overhead for the future. If the FT-450 is really different from the FT-450D in terms of cloning, you just need to subclass the existing driver with your own, and re-implement the cloning routines for it. Ideally you would parameterize the existing ones and let them examine the radio class for values (like block lengths, counts, etc). There are lots of other drivers that work this way in the tree, so there should be plenty of prior art to go on.

If you refactor this to just a delta from the existing driver, I can provide some hints for unifying duplicative cloning routines if needed.

@@ -87,7 +87,7 @@ def VALIDTONE(v):

MODES = ("WFM", "FM", "NFM", "AM", "NAM", "DV", "USB", "LSB", "CW", "RTTY",
"DIG", "PKT", "NCW", "NCWR", "CWR", "P25", "Auto", "RTTYR",
"FSK", "FSKR", "DMR", "DN")
"FSK", "FSKR", "DMR", "DN", "USER-L", "USER-U", "RTTY-L")
Copy link
Owner

Choose a reason for hiding this comment

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

These yaesu-specific "modes" need to be handled some other way. The existing driver already has problems with this (as you've probably noticed) so that needs to get fixed. I dunno what to map USER to, but RTTY-L likely needs to be just RTTY and with an extra knob that lets you select the -L variant as a separate flag or something. Or just collapse it all into RTTY and call it good :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two radio types appear to use memory differently, and they really are unique. Use of just "RTTY won't work because the different USER- types require setting different areas of memory. I already fixed my driver to handle the added types that need to be included in the potential modes in chip_common. Chirp common also burped on pep8 which is why I made the other changes. I am absolutely stuck at this point in the bowels of git and github.

@@ -929,7 +929,6 @@ def __init__(self):
self.init("has_variable_power", False,
"Indicates the radio supports any power level between the "
"min and max in valid_power_levels")

Copy link
Owner

Choose a reason for hiding this comment

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

Please don't make unrelated changes, especially just whitespace damage. I'm sure this was accidental, but it needs to be unchanged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pep8 burped on this during my local tox testing, which is probably why I made changes.

@kk7ds
Copy link
Owner

kk7ds commented Oct 26, 2023

Also, please put your bug reference in the commit message as Fixes: #nnnn so the bug system will link the fix to the issue and close it when appropriate.

Thanks for taking this on! I'm not a Yaesu person and didn't realize that the FT-450 was different than the FT-450D. You'll need to add the test report to the tests/py3_driver_testers.txt file, run tox -e makesupported and commit the result in order to pass the "support matrix" check. That will add it to the list on the front page once this is merged.

@WCurlew
Copy link
Contributor Author

WCurlew commented Oct 26, 2023

No idea how to fix the modules that need it, git won't allow me to upload any other changes as the "tip" of my branch doesn't match my local stuff anymore. No a GIT user and am getting lost in the jargon.

@WCurlew
Copy link
Contributor Author

WCurlew commented Oct 26, 2023

OK, now that I'm better "coffeed" I think I can modify the 450d driver to work with both the 450 and the 450d. I have it up and downloading OK, but here's the rub. When I download from the radio, I can check the first block to see if it is 2 or 4 bytes of data. This tells me if we have a 450 (2) or a 450d (4), If it's two, I expand it to 4 (with a 2 byte breadcrumb) and set a self.FT450 class variable for use in tweaking the code. This works great as long as I start off downloading from the radio. I have no idea how the file image open functions, so that I can check the saved image for the breadcrumb I added to the 450/d memory. Could you give me a hint as to where the file handling stuff happens, and then what do they call in the driver class to map out the file object read.

And if I should be asking this on the developers forum just let me know and I'll swap over to them.

Bill

@kk7ds
Copy link
Owner

kk7ds commented Oct 26, 2023

You should base the "2 or 4 blocks" on what radio class is being used. Meaning if they use a 450D, then use 4, and if it's 450 then 2. The same for upload. By using two separate classes, the files will be tagged as such and opening an image of a 450 will select the 450 class, and thus the upload routines will do the right thing based on the radio class.

@WCurlew WCurlew closed this Nov 4, 2023
@WCurlew WCurlew deleted the YaesuFT450Support branch November 4, 2023 01:33
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