First of many steps towards refactoring the anytone.py driver... #638
Conversation
…act the core commonality among the variant models (e.g. Anytone 5888UV vs. the Powerwerx DB-750X). This step encapsulates the communication protocol (via the "pipe.")
|
|
|
||
| def __init__( | ||
| self, pipe, block_size: int, file_ident, echos_write=True | ||
| ) -> None: |
There was a problem hiding this comment.
I think I've mentioned this on the list, but surely many times in other PRs. I absolutely detest black formatting style (as do many others). I'm sorry you've done this work in this way and will have to reformat it, but that's how it is. I have to deal with it at work, but I get to choose here. These parens on their own lines are one of the ugliest aspects of it, IMHO.
Most of the rest of the chirp code is fairly conventional python formatting. Please reformat this to look like the rest of the codebase.
There was a problem hiding this comment.
If not Black, then which tool should I use? Any special settings?
There was a problem hiding this comment.
I can't offer you a tool to automatically format the code in more conventional python and chirp style, as you know. You can start by writing code that the current checker (flake8) will allow, and not adding arbitrary vertical stacking of things that will fit perfectly fine on fewer lines. Anywhere there's a closing paren on a line by itself is an easy regex for not-okay-ness :)
| # You should have received a copy of the GNU General Public License | ||
| # along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| from builtins import bytes |
There was a problem hiding this comment.
The "no new uses of future" is because you're using this compatibility layer, but shouldn't.
There was a problem hiding this comment.
Oh, I didn't realize. That just carried over form the previous code and I didn't even notice it. Thanks.
|
I assume this is on hold until you refactor the existing anytone driver as another commit here, or add the new driver? Point being, there's no reason to merge this until something is using it. Thanks for adding model-specific tests - I wish I could get more people to do that, of course. |
|
I started committing this is in chunks per the PR rules to keep it to one conceptual unit per PR. Are you saying that you want me instead to commit the full anytone_clone.py file (including the bank model and the radio base class) and the powerwerx.py file all together in one PR? |
|
BTW, I only own a Powerwerx DB750X, not any of the other Anytones. So when I go to check in those refactored drivers, I'll need someone who does to beta test them. I was planning on asking the dev and user mailing lists when the time comes. |
|
Well, I would call "refactoring an existing anytone driver to allow for reuse" as one conceptual thing. Or a commit with common bits, followed by a commit of your new driver to use them even would be fine. What I really don't want is single commits that make multiple changes, and to a lesser degree, PRs that do a bunch of random gardening in multiple areas. Multiple related commits in a PR are totally fine. I have a 5888UV I can test with, if that's one of the ones that will be affected. Something else I did during the py3 conversion was write a clone simulator for a radio that will pass with the current code and then change the code itself. That seemed pretty accurate, and you've already demonstrated the ability to do that here, so maybe that's a strategy in the absence (or in addition to) a volunteer. |
First of many steps towards refactoring the anytone.py driver to extract the core commonality among the variant models (e.g. Anytone 5888UV vs. the Powerwerx DB-750X).
This step encapsulates the communication protocol (via the "pipe.")
This new logic will first be used in the upcoming powerwerx.py driver. The older anytone*.py drivers will be refactored after that.