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

FIX: Retevis-RB627B [Enabled Power Mode and Set Max Power to 2W (2.00)] #721

Closed
wants to merge 1 commit into from

Conversation

dguimaraes88
Copy link

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.

_pmr = True
_frs = _gmrs = _murs = False
_pmr = True
_frs = _murs = _pmr = False
Copy link
Owner

Choose a reason for hiding this comment

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

@KC9HI Does this look right to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

No. It look like they are trying to remove the PMR restrictions from the RB627B like I have been doing for other Retevis models (I got the green light from them to drop PMR, FRS, GMRS, etc restrictions allow CHIRP to fully support the radios actual capability so I have been doing it when requested).

What I simply would have done here to drop the restrictions with the least amount of change would have been to simply change line 909 _pmr = True to _pmr = False and left line 910 alone. Removing _gmrs from line 910 could inadvertently turn on GMRS restrictions on for this PMR radio.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have also been putting a comment on line 909 explaining:
_pmr = False # sold as PMR radio but supports full band TX/RX

Copy link
Contributor

@KC9HI KC9HI Jul 13, 2023

Choose a reason for hiding this comment

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

And I just remembered that the reason I didn't change this one is because the user can just use the RB27 selection and get the whole shooting match (99 channels, full band, wide/narrow bandwidth, high/low power, etc). There was no need to change RB27V or RB627B.

Copy link
Author

Choose a reason for hiding this comment

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

Ok...I understand ;) no problem, Good joob

@KC9HI
Copy link
Contributor

KC9HI commented Jul 18, 2023

The Model Alias Map change that I just submitted was related to this PR so that owners of this radio (RB627B) and its close cousins (RB27B and RB27V) know to use the RB27 to get full support. That makes this PR no long necessary.

@kk7ds kk7ds closed this Jul 19, 2023
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.

4 participants