-
Notifications
You must be signed in to change notification settings - Fork 11
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
feature: add 0x4521 DisableKeys #12
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.
I am going to be strict about the file structure, the current documentation formats are already a mess. This repo is meant to solve that. The documentation is already public on the google drive so this is not blocking anything.
I would recommend copying the IRoot
file and modifying it for this feature, instead of trying to rewrite the original documentation from scratch.
0x4521 - Disable Keys | ||
********************* | ||
|
||
Interface |
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.
This can be removed.
[0x4521] DisableKeys | ||
==================== | ||
|
||
disableableKeys = [0]GetCapabilities() | ||
|
||
disabledKeys = [1]GetDisabledKeys() | ||
|
||
disabledKeys = [2]SetDisabledKeys(keysToDisable) |
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.
Replace this with a function table, you can copy it fro IRoot
:
.. table:: Table 1 - Functions
== ====================== =======================================================
ID Name Description
== ====================== =======================================================
0 ``GetFeature`` Returns the feature index from a desired feature ID
1 ``GetProtocolVersion`` Returns the protocol version / Replies to a ping action
== ====================== ======================================================
|
||
disabledKeys = [2]SetDisabledKeys(keysToDisable) | ||
|
||
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Define the presence of the keys which the SW allows the user to disable, and allow SW to disable them. | ||
A unifying device containing this feature should adopt HID++ reset policy #3 and implement the 0x0020 | ||
configuration change feature. |
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.
This description can go above the functions table. I would also reword it a bit.
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.
This continues, there isn't supposed to be a DisabledKeys
section. Move it above the functions table, right below the main title.
+---+---+---+---------+--------+------------+---------+----------+ | ||
| 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | | ||
+===+===+===+=========+========+============+=========+==========+ | ||
| | | | Windows | Insert | ScrollLock | NumLock | CAPSLock | | ||
| | | | (Start) | | | | | | ||
+---+---+---+---------+--------+------------+---------+----------+ |
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.
Tables should be under a table
directive. This is not the correct structure for tables, for one, we use the reverse order for showing bytes.
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.
This also hasn't changed.
If the PDF documents can be made public in the Google drive then that's all that is needed to use them to build the features into Solaar and other Linux code. Requiring non-simple changes to get the information into this repository is only going to mean that there will be errors here so developers will have to go do the PDF documents anyway. |
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.
The google drive folder is public, anyone can access the documentation there.
I am requesting structural changes so that it matches this repo documentation structure. The point of this repo is to have everything documented in one standard format, instead of the mess we have today.
Documentation can go into the google drive folder without QA, here it should follow the format.
I understand thing might be frustrating, but if in the future we want a central place for documentation, it needs to happen. Anyway, nothing is being blocked by this, the information is still on google drive.
@@ -0,0 +1,100 @@ | |||
********************* | |||
Disable Keys (``0x4521``) | |||
********************* |
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.
Please adjust the decorators size.
== ====================== ======================================================= | ||
ID Name Arguments | ||
== ====================== ======================================================= | ||
0 GetCapabilities None | ||
1 GetDisabledKeys None | ||
2 SetDisabledKeys keysToDisable | ||
== ====================== ======================================================= |
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.
That is not what this table is supposed to be. It should provide a description of the function. The arguments are provided in the specific function section.
Define the presence of the keys which the SW allows the user to disable, and allow SW to disable them. | ||
A unifying device containing this feature should adopt HID++ reset policy #3 and implement the 0x0020 | ||
configuration change feature. |
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.
This continues, there isn't supposed to be a DisabledKeys
section. Move it above the functions table, right below the main title.
+---+---+---+---------+--------+------------+---------+----------+ | ||
| 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | | ||
+===+===+===+=========+========+============+=========+==========+ | ||
| | | | Windows | Insert | ScrollLock | NumLock | CAPSLock | | ||
| | | | (Start) | | | | | | ||
+---+---+---+---------+--------+------------+---------+----------+ |
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.
This also hasn't changed.
I fixed the format issue, but someone else is going to have to take over at this point. |
Produced from PDF file for feature