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.

Sign up for GitHub

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

Jump to bottom

UV-5R: fine tune Aux block upload restrictions - related to #10505 #726

Merged
merged 1 commit into from
Jul 14, 2023
Merged

UV-5R: fine tune Aux block upload restrictions - related to #10505 #726

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 91 additions & 62 deletions chirp/drivers/uv5r.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,10 @@
txpower3:2;
} vfob;

#seekto 0x0F56;
u16 fm_presets;
#seekto 0x0F48; // some new radios do not support this feature
char bcstfmlo[14]; // broadcast FM 65-75 MHz band
u16 fm_presets; // broadcast FM frequency
char bcstfmhi[14]; // broadcast FM 75-108 MHz band
Copy link
Owner

@kk7ds kk7ds Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh I see you added these elements here just now, I got it.

I guess maybe these could be different in some radios for other geographies maybe? I guess we'll just have to see, but if the new radios are the only ones with 0xFF in them, then we'll catch those anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I only have 1 radio here with the issue. But the 2 or 3 images that I have gotten from others all have bcstfmlo[14] and bcstfmhi[14] fully populated with 0xFF. So far I have never seen an 0xFF in an older "good" radio so I just check for a single 0xFF when uploading. I would rather err on the side of not uploading to a "good" radio than accidentally upload to a "bad" one.


#seekto 0x1008;
struct {
Expand Down Expand Up @@ -473,7 +475,10 @@ def _get_radio_firmware_version(radio):
else:
block1 = _read_block(radio, 0x1E80, 0x40, True)
block2 = _read_block(radio, 0x1EC0, 0x40, False)
block3 = _read_block(radio, 0x0F40, 0x40, False)
version = block2[48:62]
brdcstfm = block3[0:30]
return version, brdcstfm
return version


Expand Down Expand Up @@ -517,7 +522,10 @@ def _ident_radio(radio):
def _do_download(radio):
data = _ident_radio(radio)

radio_version = _get_radio_firmware_version(radio)
if radio.MODEL == "BJ-UV55":
radio_version = _get_radio_firmware_version(radio)
else:
radio_version, radio_fm = _get_radio_firmware_version(radio)
LOG.info("Radio Version is %s" % repr(radio_version))

if b"HN5RV" in radio_version:
Expand Down Expand Up @@ -591,55 +599,71 @@ def _do_upload(radio):
raise errors.RadioError("Image not supported by radio")

image_version = _firmware_version_from_image(radio)
radio_version = _get_radio_firmware_version(radio)
if radio.MODEL == "BJ-UV55":
radio_version = _get_radio_firmware_version(radio)
else:
radio_version, radio_fm = _get_radio_firmware_version(radio)
LOG.info("Image Version is %s" % repr(image_version))
LOG.info("Radio Version is %s" % repr(radio_version))

imagev_matched_radiov = image_version == radio_version
_skip_aux_block = False

# default ranges
_ranges_main_default = [
(0x0008, 0x0CF8),
(0x0D08, 0x0DF8),
(0x0E08, 0x1808)
]
_ranges_aux_default = [
(0x1EC0, 0x1EF0),
(0x1EC0, 0x1EF0) # 6 key power-on & welcome messages
]

# skip broadcast fm range
_ranges_main_no_fm = [
(0x0008, 0x0CF8),
(0x0D08, 0x0DF8),
(0x0E08, 0x0F48),
(0x0F68, 0x1808)
]

# extra aux ranges
_ranges_aux_extra = [
(0x1F60, 0x1F70),
(0x1F80, 0x1F90),
(0x1FC0, 0x1FE0)
(0x1F60, 0x1F70), # squelch thresholds - vhf
(0x1F80, 0x1F90), # squelch thresholds - uhf
(0x1FC0, 0x1FE0) # band limits
]

if b"HN5RV" in radio_version:
# not safe to upload 'aux block', skip it.
# set default ranges
ranges_main = _ranges_main_default
ranges_aux = _ranges_aux_default

# check destinaton radio for invalid bcst FM data
if b"\xFF" in radio_fm:
# 0x0F48-0x0F65 contains invalid data
# This radio is likely to 'break' the Aux block if it is changed.
# The safest thing to do is to skip uploading of the Aux block.
ranges_main = _ranges_main_no_fm
_skip_aux_block = True
else:
_skip_aux_block = False
# check source image for invalid bcst FM data
elif "\xFF" in str(radio._memobj.bcstfmlo) or \
"\xFF" in str(radio._memobj.bcstfmhi):
Copy link
Owner

Choose a reason for hiding this comment

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

Okay and this is the check for upload.

# 0x0F48-0x0F65 contains invalid data
# This image appears to be from a radio that was likely to 'break'
# the Aux block if it is changed (and could be broken).
# The safest thing to do is to skip uploading of the Aux block.
ranges_main = _ranges_main_no_fm
_skip_aux_block = True
# check if radio and image firmware versions match
elif imagev_matched_radiov:
# matched - allow all safe Aux block ranges
ranges_aux = _ranges_aux_default + _ranges_aux_extra

if radio._all_range_flag:
_skip_aux_block = False
image_matched_radio = True
ranges_main = radio._ranges_main
ranges_aux = radio._ranges_aux
LOG.warning('Sending all ranges to radio as instructed')
elif image_version == radio_version:
image_matched_radio = True
ranges_main = _ranges_main_default
ranges_aux = _ranges_aux_default + _ranges_aux_extra
elif any(type in radio_version for type in radio._basetype):
image_matched_radio = False
ranges_main = _ranges_main_default
ranges_aux = _ranges_aux_default
else:
msg = ("The upload was stopped because the firmware "
"version of the image (%s) does not match that "
"of the radio (%s).")
raise errors.RadioError(msg % (image_version, radio_version))

if not radio._aux_block:
image_matched_radio = True

# Main block
mmap = radio.get_mmap().get_byte_compatible()
Expand All @@ -663,20 +687,21 @@ def _do_upload(radio):
addr = 0x1808 + (i - 0x1EC0)
_send_block(radio, i, mmap[addr:addr + 0x10])

if not image_matched_radio:
msg = ("Upload finished, but the 'Other Settings' "
"could not be sent because the firmware "
"version of the image (%s) does not match "
"that of the radio (%s).")
raise errors.RadioError(msg % (image_version, radio_version))

if radio._all_range_flag:
radio._all_range_flag = False
LOG.warning('Sending all ranges to radio has completed')
raise errors.RadioError(
"This is NOT an error.\n"
"The upload has finished successfully.\n"
"Please restart CHIRP.")
return

if radio._aux_block and not imagev_matched_radiov:
msg = ("This is NOT an error. The upload finished successfully.\n"
"The 'Other Settings' and 'Service Settings' were skipped "
"because the firmware version of the image (%s) does not "
"match that of the radio (%s).")
raise errors.RadioError(msg % (image_version, radio_version))
Copy link
Owner

Choose a reason for hiding this comment

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

We need to figure out a better way to do this. I need to do some thinking on it, so don't let me forget.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. The problem here is that Baofeng pretty much stopped separating the various models by using different magics. So it is entirely possible to upload an image from a dual-band radio into a tri-band radio hosing the band limits. It is a similar situation between radios with 2 TX power levels and 3 TX 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.

What I mean is a better way to raise a message to the user after cloning that isn't an error. So that you don't have to say "ERROR: This is not an error" :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I've never liked this. People see the error symbol and never read what it says and always thing something bad happened. You are the one that suggested that I add the "This is not an error" part.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, I remember. I'll do some thinking.



UV5R_POWER_LEVELS = [chirp_common.PowerLevel("High", watts=4.00),
Expand Down Expand Up @@ -1104,14 +1129,17 @@ def _my_upper_band(self):
return band_tag

def _get_settings(self):
_mem = self._memobj
_ani = self._memobj.ani
_fm_presets = self._memobj.fm_presets
_settings = self._memobj.settings
_squelch = self._memobj.squelch_new
_vfoa = self._memobj.vfoa
_vfob = self._memobj.vfob
_wmchannel = self._memobj.wmchannel
HN5RV = "HN5RV" in str(self._memobj.firmware_msg.line1)
_has_fmradio = (str(_mem.bcstfmlo) == " FM 65-75M" and
str(_mem.bcstfmhi) == " FM 76-108M")
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, these strings are in the memory image and not changeable by the user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They not changeable by the typical CHIRP user because they are not exposed to the user. But they are not immutable like the firmware version is in Aux memory block, if that's what you mean. They could be changed by someone that really wanted to change them.

It is for this reason that I block the upload of this region of memory if either or both the image or radio does not have these strings so they can't get overwritten by normal CHIRP use.


basic = RadioSettingGroup("basic", "Basic Settings")
advanced = RadioSettingGroup("advanced", "Advanced Settings")

Expand Down Expand Up @@ -1431,7 +1459,7 @@ def _filter(name):
'to them. Please see bug #10505 on the CHIRP '
'issue tracker for more details.')
for setting in aux_settings:
if HN5RV:
if not _has_fmradio:
setting.set_warning(hn5rv_warn)
other.append(setting)

Expand Down Expand Up @@ -1608,29 +1636,30 @@ def apply_offset(setting, obj):
STEP_LIST, STEP_LIST[_vfob.step]))
workmode.append(rs)

fm_preset = RadioSettingGroup("fm_preset", "FM Radio Preset")
group.append(fm_preset)

# broadcast FM settings
value = self._memobj.fm_presets
value_shifted = ((value & 0x00FF) << 8) | ((value & 0xFF00) >> 8)
if value_shifted >= 65.0 * 10 and value_shifted <= 108.0 * 10:
# storage method 3 (discovered 2022)
self._bw_shift = True
preset = value_shifted / 10.0
elif value >= 65.0 * 10 and value <= 108.0 * 10:
# storage method 2
preset = value / 10.0
elif value <= 108.0 * 10 - 650:
# original storage method (2012)
preset = value / 10.0 + 65
else:
# unknown (undiscovered method or no FM chip?)
preset = False
if preset:
rs = RadioSettingValueFloat(65, 108.0, preset, 0.1, 1)
rset = RadioSetting("fm_presets", "FM Preset(MHz)", rs)
fm_preset.append(rset)
if _has_fmradio:
fm_preset = RadioSettingGroup("fm_preset", "FM Radio Preset")
group.append(fm_preset)

# broadcast FM settings
value = self._memobj.fm_presets
value_shifted = ((value & 0x00FF) << 8) | ((value & 0xFF00) >> 8)
if value_shifted >= 65.0 * 10 and value_shifted <= 108.0 * 10:
# storage method 3 (discovered 2022)
self._bw_shift = True
preset = value_shifted / 10.0
elif value >= 65.0 * 10 and value <= 108.0 * 10:
# storage method 2
preset = value / 10.0
elif value <= 108.0 * 10 - 650:
# original storage method (2012)
preset = value / 10.0 + 65
else:
# unknown (undiscovered method or no FM chip?)
preset = False
if preset:
rs = RadioSettingValueFloat(65, 108.0, preset, 0.1, 1)
rset = RadioSetting("fm_presets", "FM Preset(MHz)", rs)
fm_preset.append(rset)

dtmf = RadioSettingGroup("dtmf", "DTMF Settings")
group.append(dtmf)
Expand Down Expand Up @@ -1743,7 +1772,7 @@ def apply_code(setting, obj):
RadioSettingValueInteger(
0, 123,
getattr(_obj, "sql%i" % (index))))
if HN5RV:
if not _has_fmradio:
rs.set_warning(hn5rv_warn)
service.append(rs)

Expand Down