miracle2k / onkyo-eiscp

Control Onkyo A/V receivers over the network; usuable as a script, or as a Python library.
MIT License
467 stars 110 forks source link

fix commands for SLI23 : select source CD or TV/CD #61

Closed jeromeloman closed 7 years ago

jeromeloman commented 7 years ago

earlier model of onkyo don't have a TV source or CD source they have only a "TV/CD" source so I think it makes sense for them to query "TV/CD" or "CD" directly...

danieljkemp commented 7 years ago

It looks like the change should also be made in commands.yaml. My understanding is that the excel API documentation is parsed, then cleaned up to generate the commands.yaml, and from there commands.py is generated.

The script that generates commands.py includes this directive

# This file can and should be manually changed to fix things the
# automatic import didn't and often can't do right. These changes
# should be tracked in source control, so they can be merged with
# new generated versions of the file.

But it is not clear to me where those individual tweaks are tracked.

jeromeloman commented 7 years ago

oh indeed, looks like you just update commands.yaml, then each time we regenerate commands.yaml we have to merge the manual tweak back in.

miracle2k commented 7 years ago

The idea is that by tracking the changes in git, we can go back and do a three-way merge between common-base, new generated output, and manual tweaks. The one and only time I did that it worked reasonably well, but it might be better to split the YAML by command into individual files.

As for this change, should it maybe be "tv/cd", "tv", and "cd"?

danieljkemp commented 7 years ago

The issue we encountered is that when a receiver has both a plain 'tv' input, and a 'tv'/cd' input, when selecting 'tv' the code matches the 'tv' from 'tv'/cd' rendering the plain 'tv' input unreachable.

On Fri, 7 Apr 2017, 17:18 Michael Elsdörfer, notifications@github.com wrote:

The idea is that by tracking the changes in git, we can go back and do a three-way merge between common-base, new generated output, and manual tweaks. The one and only time I did that it worked reasonably well, but it might be better to split the YAML by command into individual files.

As for this change, should it maybe be "tv/cd", "tv", and "cd"?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/miracle2k/onkyo-eiscp/pull/61#issuecomment-292654271, or mute the thread https://github.com/notifications/unsubscribe-auth/AJzfuQlWqiicez5K4cz5PsJr9BonSiOKks5rtqgdgaJpZM4M3Dse .

miracle2k commented 7 years ago

I see. I'll merge a pull request!

joe248 commented 7 years ago

@miracle2k what's the status on merging this?

For reference, here's the issue I opened for this: https://github.com/miracle2k/onkyo-eiscp/issues/60

miracle2k commented 7 years ago

This pull request isn't quite right, so I can't merge it; the change needs to go into the .yaml file, since the commands.py is generated.

joe248 commented 7 years ago

@miracle2k what's the status on this? I can submit a PR if you want. I believe all that needs to change in the .yaml file is that lines like this:

name: [cd, tv, cd]

should be;

name: [cd, tv/cd]

Is that correct?

miracle2k commented 7 years ago

Yes!