Conversation
Add support for Wouxun KG-UV950P Fixes: #10498
| out_str = '' | ||
| for c in in_str: | ||
| if c > 95: | ||
| out_str += '' |
There was a problem hiding this comment.
This is not free but has no effect. Just replace this line with pass
| elif c >= 80: | ||
| out_str += chr(c-48) | ||
| else: | ||
| out_str += chr(c+48) |
There was a problem hiding this comment.
Can you explain what you're doing here? It kinda looks like maybe you're just needing to mask out 0x80 from each of these characters, but I'm not sure what you're trying to accomplish here. Is the character set in the radio just starting at zero (for the character 0)? Maybe 0x80 is just a flag for "capitals" or something? Either way, doing this with magic decimal numbers doesn't make this very understandable by other people with no comments.
| else: | ||
| out_str += chr(int(ord(c) - 48)) | ||
| while len(out_str) < 8: | ||
| out_str += chr(0x50) |
There was a problem hiding this comment.
This while loop can be replaced by this:
out_str = out_str.ljust(8, chr(0x50))
| out_str += chr(int(ord(c) - 48)) | ||
| while len(out_str) < 8: | ||
| out_str += chr(0x50) | ||
| if out_str == " " or out_str == "": |
There was a problem hiding this comment.
And this can be:
if not out_str.strip():
The next person that comes along will have to count how many spaces in that string trigger this special behavior, but the above makes it clear that it's "if it's all spaces, use this special thing".
| mem.name = _str_decode(self._memobj.names[number].name) | ||
| # Remove trailing whitespaces from the name | ||
| mem.name = mem.name.rstrip() | ||
| if self.MODEL != "KG-UV950P": |
There was a problem hiding this comment.
Just override _str_decode() on the classes, and make the 950 subclass to do the thing for the 950. That's the reason for the class structure - if you subclass something, everything is inherited unless it is overridden and this is exactly the sort of thing that should be done that way. We honestly have too much of this if self.MODEL stuff in CHIRP. It's quicker to just do it that way in the middle of a complex function (like get_settings()) but for something like this that is a small unit of work that can be overridden, we should do that.
Looking around in this file, it looks like we've collected a lot of this exact pattern, so I think we need to clean that up here and not make it worse.
| else: | ||
| mem.name = _str_decode_950(self._memobj.names[number].name) | ||
| # Remove trailing whitespaces from the name | ||
| mem.name = mem.name.rstrip() |
There was a problem hiding this comment.
Just do this once at the end instead of repeating the comment and the rstrip() :)
| vfoa_grp.append(vfo450_grp) | ||
| if self.MODEL == "KG-UV980P": | ||
| if (self.MODEL == "KG-UV980P" or | ||
| self.MODEL == "KG-UV950P"): |
There was a problem hiding this comment.
This is the sort of thing where I'm willing to go along with the use of the model thing, because it's in the middle of a larger block of stuff that's not easy to swap out. But for the str encode/decode you've already got those as separate methods, so you should use the class hierarchy to do the thing for you.
| else: | ||
| temp = c + 48 | ||
| if chr(temp) in chirp_common.CHARSET_ASCII: | ||
| _str += chr(temp) |
There was a problem hiding this comment.
Is this just repeated from str_decode() or one of the many other variants you have in this file? Even if it's not, why put it here instead of with all the others?
| if self.MODEL != "KG-UV950P": | ||
| _str = _decode(eval("_settings.scanname"+x)) | ||
| else: | ||
| _str = _decode_950(eval("_settings.scanname"+x)) |
There was a problem hiding this comment.
Here again, this should be just self._decode() and override it on the subclasses that need it.
2 participants
Add support for Wouxun KG-UV950P
Fixes #10498
CHIRP PR Checklist
The following must be true before PRs can be merged:
tests/images(except for thin aliases where the driver is sufficiently tested already).Please also follow these guidelines:
six,future, etc).NEEDS_COMPAT_SERIAL=Falseand useMemoryMapBytes.tox -emakesupportedand include it in your commit.