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 RT490 - fixes #9665 #814

Merged
merged 3 commits into from
Nov 24, 2023
Merged

Add Radtel RT490 - fixes #9665 #814

merged 3 commits into from
Nov 24, 2023

Conversation

KC9HI
Copy link
Contributor

@KC9HI KC9HI commented Nov 20, 2023

Submitted on behalf of [email protected]

Also includes the following models:
MMLradio JC-8629
JJCC JC-8629
Socotran JC-8629
Socotran FB-8629
Jianpai 8800 Plus
Boristone 8RS
Abbree AR-869
HamGeek HG-590

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.

@kk7ds
Copy link
Owner

kk7ds commented Nov 20, 2023

I dunno why the check thinks there is no image, so ignore that. The complaint about the dedent usage inside the _( translation marker is legit, but I can fix that if you want. I don't think I've looked over this before so it'll take me a bit to get through it all. Thanks for picking this up!

@KC9HI
Copy link
Contributor Author

KC9HI commented Nov 20, 2023

The only part that I know how to fix is the 'match_model()'. So if you would take care of it, I would appreciate it.

@KC9HI
Copy link
Contributor Author

KC9HI commented Nov 20, 2023

Maybe it wants an image per model instead of per driver?

@kk7ds
Copy link
Owner

kk7ds commented Nov 20, 2023

No, it's not that smart of a script :)

@kk7ds
Copy link
Owner

kk7ds commented Nov 20, 2023

Oh it's because I fixed this bug already and it's in another PR waiting for merge. I'll slide that in front here when I fix the dedent thing. I'll wait until you push up the match model bit.

@KC9HI
Copy link
Contributor Author

KC9HI commented Nov 20, 2023

Looks like the match_model() thing was as easy as I thought it would be. Let me see if I can figure it out.

@kk7ds
Copy link
Owner

kk7ds commented Nov 20, 2023

Oops, sorry I didn't see that until I pushed.. I think it should be fixed now. I probably won't review this tonight but will try tomorrow. Thanks for getting it submitted!

@kk7ds
Copy link
Owner

kk7ds commented Nov 20, 2023

The license check will still fail because of that wording in the comments, but as noted, that's okay and doesn't affect this file's license.

@kk7ds
Copy link
Owner

kk7ds commented Nov 20, 2023

Gah, I'll look at the detection issue too.

@KC9HI
Copy link
Contributor Author

KC9HI commented Nov 20, 2023

After removing match_model(), I got this exception.

E Exception: Detection of C:\Users\Jim\mychirpgit\chirp\tests\unit..\images\BTECH_GMRS-50X1.img failed: [<class 'chirp.drivers.btech.GMRS50X1'>, <class 'chirp.drivers.radtel_rt490.RT490Radio'>, <class 'chirp.drivers.radtel_rt490.MML8629Radio'>, <class 'chirp.drivers.radtel_rt490.JJCC8629Radio'>, <class 'chirp.drivers.radtel_rt490.SocotranJC8629Radio'>, <class 'chirp.drivers.radtel_rt490.SocotranFB8629Radio'>, <class 'chirp.drivers.radtel_rt490.Jianpai8629Radio'>, <class 'chirp.drivers.radtel_rt490.Boristone8RSRadio'>, <class 'chirp.drivers.radtel_rt490.AbbreeAR869Radio'>, <class 'chirp.drivers.radtel_rt490.HamGeekHG590Radio'>]

@kk7ds kk7ds force-pushed the add-rt490 branch 2 times, most recently from 2eb6145 to 86f1457 Compare November 24, 2023 19:20
KC9HI and others added 3 commits November 24, 2023 11:22
Submitted on behalf of [email protected]

Also includes the following models:
	MMLradio JC-8629
	JJCC JC-8629
	Socotran JC-8629
	Socotran FB-8629
	Jianpai 8800 Plus
	Boristone 8RS
	Abbree AR-869
	HamGeek HG-590
We need to stop implementing this, but it should be defaulted to
just "return False" but we don't have a super good way to do that
really.
@kk7ds
Copy link
Owner

kk7ds commented Nov 24, 2023

The PR check is just complaining about the dual-license statement, which is not a problem, so will merge anyway.

@kk7ds kk7ds merged commit 3c1ff5b into kk7ds:master Nov 24, 2023
@KC9HI KC9HI deleted the add-rt490 branch December 1, 2023 19:56
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