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.

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

Review rxv.py and refactor #31

Open
2 tasks
wuub opened this issue Nov 10, 2016 · 2 comments
Open
2 tasks

Review rxv.py and refactor #31

wuub opened this issue Nov 10, 2016 · 2 comments

Comments

@wuub
Copy link
Owner

wuub commented Nov 10, 2016

TODO:

  • tasks here
  • and here
@sdague
Copy link
Contributor

sdague commented Nov 10, 2016

Here are some random ideas on breaking up the code a bit. They might be a bridge too far in refactoring.

  1. The long list of command strings that are just at the beginning of the file could be turned into small class objects, in a dedicated file.

For instance:

class PlayGet(RXVRequest):
   zone_cmd = False
   method = "GET"

   def __init__(self, source):
       self.source = source

   def __str__(self, zone=None):
       return "<{src_name}><Play_Info>GetParam</Play_Info></{src_name}>".format(self.source)

Then calling code would look like:

resp = self._request(PlayGet(source))

We could have a PlayGetResponse class that's responsible for the parsing (and also a good place in the code to document what the return document is going to look like (I've been writing some notes locally, but a standard place to do that in code would be nice).

That takes a bunch of internal complexity out of the methods in the main class, and makes it more of high level flow:

def do_thing(self):
    if not can_do_thing(self.input):
        bail
    resp = self._request(Thing(self.input))
    return resp
  1. move static discovery into a dedicated module, though I'm not sure I really have a good handle on the exposed interface because we both want it for safety checks in our methods, but we are also going to want a dump or something to support systems like HomeAssistant setting the capabilities on the receiver when that interface is active (which impacts the controls shown to the user).

@sdague
Copy link
Contributor

sdague commented Nov 10, 2016

Also, totally random bonus points if someone cracks the yamaha icon format that exists as part of the status. There is definitely a bitmap in there somewhere, as it's how both the app and the TV ui display an icon for net radio stations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants