th_uv88 : optional signaling implementation #11110 #916
Conversation
- dtmf - 2 tones - 5 tones update test image file to have a wider variety of settings
…l signaling channels settings.
chirp/drivers/th_uv88.py
Outdated
| # Encode channels | ||
| for i in range(16): # 0 - 15 | ||
| sigchan = RadioSettingSubGroup("dtmfencchan%d" % i, | ||
| "Channel %02d" % (i+1)) |
There was a problem hiding this comment.
Nice use of SubGroup. This is new, I just added it recently, but I think it really helps organize things better. Note that the "FM radio" section of this radio could probably use some similar cleanup, if you feel like it :)
I wish the style checker would complain, but python conventional style is space around operators (so i + 1), with = being the exception when used in parameter lists.
There was a problem hiding this comment.
I found the SubGroup while looking at recent commits :) Yes FM settings would also benefit the use of subgroup. I will add this to the #11153 issue
just commited the style fix
chirp/drivers/th_uv88.py
Outdated
|
|
||
| def _set_int10(setting, obj, atrb): | ||
| vx = float(str(setting.value)) | ||
| vx = int(vx * 10) |
There was a problem hiding this comment.
I don't think you should need these int and float casts here. If you do, maybe there's some work that needs doing elsewhere.
There was a problem hiding this comment.
You are right. Removing the cast do the job.
chirp/drivers/th_uv88.py
Outdated
| if bool(setting.value): # Enabled = 1 | ||
| vx = 1 | ||
| else: | ||
| vx = 0 |
There was a problem hiding this comment.
I think you should be able to just int(setting.value) or just use setting.value without the cast, no? Not sure if you did this out of habit or need, but let me know and I could maybe make some more native fixes so they behave properly.
There was a problem hiding this comment.
just copy pasted this code from the myset_mask (used in fm settings). I think this is to be sure that vx is either 0 or 1 because the _do_map function accept also a 2 value which just return the state.
using bool(setting.value) is working and I think it guarantee a value of 0 à 1
There was a problem hiding this comment.
Sure, but int(bool) in python will give you a 0 or 1 and save you three lines. Anyway, I know it's copy/paste, just thought I'd point it out since you asked about pythonic code style :)
|
This is still marked as draft, so I've been leaving it alone. No pressure, but if/when you're ready for it to merge just un-draft it . |
|
I just undrafted it as it works well in the current state. I only own a TH-UV88 and got no feedback for RA89/QRZ-1. While those radios ae very close I was not able to do tests. |
Add settings to configure :
updated test image file to have a wider variety of settings