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

Insert of new findings #4

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

BasVanBoven
Copy link

Ordering of sections could probably still be improved, but this will probably do content-wise

Copy link
Owner

@therealmuffin therealmuffin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Bas,

First of all, apologies for the delayed response. Second, thanks for your research and great addition to the documentation, very useful stuff!

I've read through the changes you've made and most of it looks fine. However, I think that there are some details missing in section 2.2 which are required to correctly understand that part (see the inline comments). Can you take a look at that subsection? If you want, I can also pick that one up (as long as you can give me some input on how to interpret that section - and if perhaps my suspicion is correct or not).


## 3. Feedback

Feedback is slightly different and carries a payload of 8 bytes instead of 5. A number of examples follow:
 

type | Command Code (CC) | Command Value (CV) | payload structure | payload example
type | Command Code (CC) | Command Value (CV) | payload structure | feedback example
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you revert this change so it says "payload example"? I believe that's the more appropriate term since this column only shows the payload, not an entire feedback message.

### 2.1. Shortcuts

Not all that is specified is necessary. While volume commands have a specific command code for volume up and down, when specifying absolute volume commands (is there another way?), this distinction can be ignored.

The status value supplied with most commands is only partially processed. It is necessary to specify the correct zone. However, the input part of the status value is ignored.

### 2.2. List commands

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great addition, but I do have a couple of questions. Perhaps you can provide a little more detail on this.

  1. I see you've listed the Command Code but not the Command Value. Is that byte missing for this command? If so, is the payload 4 bytes instead of 5?
  • Looking back at my raw data, I suspect the command is "0xFF 0x55 0x01 0x1E 0xE1". If so, the command structure is completely different compared to what is discussed earlier in section 2. In that case, I think this needs to be in a separate section, just to make the distinction clear. Also, since it is not really a control command. It's more like a command to retrieve Device Information.
  1. I see that the Command Code for Device Properties is 0x01. However, that's the same code as for Power On. Are you sure this is correct?
  • If my suspicion mentioned above is correct, this isn't really strange...

@therealmuffin therealmuffin linked an issue Jun 5, 2021 that may be closed by this pull request
@BasVanBoven
Copy link
Author

BasVanBoven commented Jun 5, 2021 via email

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

Successfully merging this pull request may close these issues.

Requesting feedback
2 participants