-
Notifications
You must be signed in to change notification settings - Fork 1
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 |
There was a problem hiding this comment.
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 | ||
|
There was a problem hiding this comment.
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.
- 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.
- 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...
Hi Maarten, just a quick reply: your assumption is correct :). I can pick
up the rest when I find some time again. Regards, Bas
…On Sat, 5 Jun 2021, 21:13 Muffinman, ***@***.***> wrote:
***@***.**** requested changes on this pull request.
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).
------------------------------
In README.md
<#4 (comment)>
:
>
## 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
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.
------------------------------
In README.md
<#4 (comment)>
:
> ### 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
+
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...
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGZANJ4KHYYCFRMS6LFLPLTRJZMRANCNFSM44SHVYGQ>
.
|
Ordering of sections could probably still be improved, but this will probably do content-wise