Ftrollup #1396
Ftrollup #1396
Conversation
|
The old "fixdigi" commit for Yaesu FT-xD is part of this rollup, as are the gardening changes, each in separate commits. The seekto changes will still have the occasional "unnecessary", but no more backwards. And they don't break FTM-yD radios. I finally got an inkling of what David Bouch was trying to talk me through (2 years ago, now!) about cherry-picking commits into a new branch. Hence the new ftrollup commit. I hope this is all OK. It passed the automated tests. |
|
The UI line with RadioSettingSubGroup still gets mousover errors. |
|
Moved the category name setting into the SubGroup. |
|
As noted on the bug you opened, I can't reproduce the mouseover errors. Also, I'm not sure if this is actually supposed to be reviewed because it contains commits that claim to break some of the drivers, for deprecation reasons I replied on list were not necessary to address. Are you saying that the first commit breaks them and the second re-fixes them? If so, we definitely don't want to put that breakage in the history, so please squash them. |
|
Thanks. I realize my error; I'd cherry-picked one commit too many (it was historical: first commit worked but left broken things in dependent drivers, second one fixed that. I think I should have only used the second commit in the rollup.) How do you recommend I 'squash' that out? I can rebuild that branch with the correct commits; will that upset the history-keeping? That's easy. Should I cancel the PR and resubmit? Or, since it's all in one driver file, just let github figure it out when I push a new version? |
|
Does it need to be squashed or removed? Either way, |
chirp/drivers/ft1d.py
Outdated
| // GM / Digital | ||
| // already at 0x04c0; | ||
| // Caution: overloaded at 04c1! Code change necessary |
There was a problem hiding this comment.
Is the "code change necessary" the one you're making in this commit, or are you saying another change is needed that this does not account for?
chirp/drivers/ft1d.py
Outdated
| val = RadioSettingValueString(0, 16, msg) | ||
| rs = RadioSetting("opening_message.message.padded_yaesu", | ||
| "Opening Message", val) | ||
| "Opening Message (16 chars)", val) |
There was a problem hiding this comment.
Let's not get into this pattern here. I can make a change to the UI that will put the required length into the tooltip (or something) that will work the same for all string settings, okay?
| mem.duplex = DUPLEX[""] | ||
| else: | ||
| mem.duplex = DUPLEX[_mem.duplex] | ||
| mem.duplex = DUPLEX[_mem.duplex] if _mem.duplex else "" |
There was a problem hiding this comment.
....still would like to know how this is related.
There was a problem hiding this comment.
I really don't understand why you won't explain this change. The previous code was obviously wrong, but fixing it should be in a separate commit from this with an explanation.
|
Begin forwarded message:
From: Dan Smith ***@***.***>
Subject: Re: [kk7ds/chirp] Ftrollup (PR #1396)
Date: September 21, 2025 at 16:37:08 GMT
To: kk7ds/chirp ***@***.***>
Cc: Declan Rieb ***@***.***>, Author ***@***.***>
Reply-To: kk7ds/chirp ***@***.***>
@kk7ds commented on this pull request.
<snip: handled inline. I finally got back to this computer with all the changes.>
Let’s not get into this pattern here. I can make a change to the UI that will put the required length into the tooltip (or something) that will work the same for all string settings, okay?
OK, I’ll have to learn about .set_doc, I guess. Just not now.
These particular settings take up the whole width of the screen, so a few extra characters on the line are pretty much a wash.
Also, the setting title and the setting value itself each take up half the line width. Some values can be longer, but I cannot increase the length of that side of the line (or, equivalently, shrink the title space) to be able to see more of the value.
In chirp/drivers/ft1d.py <#1396 (comment)>:
> @@ -1056,10 +1056,7 @@ def get_memory(self, number):
mem.offset = int(_mem.offset) * 1000
mem.rtone = mem.ctone = chirp_common.TONES[_mem.tone]
self._get_tmode(mem, _mem)
- if mem.duplex is None:
- mem.duplex = DUPLEX[""]
- else:
- mem.duplex = DUPLEX[_mem.duplex]
+ mem.duplex = DUPLEX[_mem.duplex] if _mem.duplex else ""
....still would like to know how this is related
As I indicated before, my particular live setup needed simplex. The former logic was all wrong AND provided incorrect field (mem.duplex depended upon mem.duplex NOT _mem.duplex for the if-check.
…______
Declan Rieb WD5EQY
***@***.***
|
|
Oh no! what have I done now and how? When locally switched to ftrollup, the git log shows a latest-commit as Sep 25 (344e850). When switched to master, the git log shows a latest-commit at Oct 8 (abd542d) Sigh, |
|
Finally... No more merge. All tests passed. I had to do the rebase FIRST. It wasn't obvious that the github.com error message also had a drop-down menu that suggested either merge or rebase. whew! |
|
As noted in the PR guidelines your last commit to fix stuff "after comments" should be squashed. I really wanted the duplex fix in a separate commit and explained. But, I'm out of ideas for trying to get you to answer me. So, since this needs to be squashed and rebased I'll just split that bit out myself. |
This could never have worked, and its origin is unknown. So remove it.
…category. Fixes #12108.
2 participants
CHIRP PR Guidelines
Yaesu FT-1D, FT2D, FT3D fixes WiresX settings to handle all rooms in category.
Also has gardening fixes in separate commits: removes reverse seekto, text-handling in settings, helper-function naming.