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

Configuration guide #173

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

gonX
Copy link
Member

@gonX gonX commented Mar 15, 2024

Preview: image

Looking for fact checking, theming issues and TODO fixes.

Necessary fixes before merge:

  • h4/h5 theming

@gonX gonX added enhancement New feature or request wiki Improvements or additions to the wiki labels Mar 15, 2024
Copy link

@JaxCavalera JaxCavalera left a comment

Choose a reason for hiding this comment

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

This is awesome thank you for taking the time to put this together!


### MacOS

TODO, probably just uninstall existing drivers?

Choose a reason for hiding this comment

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

Quite likely it'll be nearly the same as Linux, or easier 🙏 Hopefully someone that uses Mac can add to this.

Copy link
Member

@jamesbt365 jamesbt365 left a comment

Choose a reason for hiding this comment

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

While this guide does not directly cover stuff like WinUSB, examples of bad/unopenable lengths and how to handle situations where multiple interfaces have the same lengths (interface attribute required), this is good!

It covers most bases and I don't really have much to be picky about, but heres some stuff I could think of while reading this.


### MacOS

TODO, probably just uninstall existing drivers?
Copy link
Member

Choose a reason for hiding this comment

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

As long as OTD was installed properly, the relevant permissions should already be present, so the only thing that needs to be done is uninstall the OEM driver.

Unless I'm missing something? But even if I am I feel like mentioning something is better than just a TODO.

Copy link
Member Author

Choose a reason for hiding this comment

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

After lurking some more my observations are the same. So will adjust this to just uninstall existing drivers.

Comment on lines +105 to +106
Note that wheels (rings) are not yet supported, and should not be directly defined
(even as a button). See [OpenTabletDriver#1367] for more information
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is clear enough that auxiliary buttons at the center of the wheel are fine to treat as normal auxiliary keys (as they are usually normal auxiliary keys)

Copy link
Member Author

Choose a reason for hiding this comment

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

Historically we've requested that center wheel keys not to be defined as an aux key. This is to ease future wheel implementations which likely expect a button.

If they are defined as an aux key anyway, there should be a code comment indicating which field is the wheel button (this is not convention yet but would be nice to do)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'll change this to allow defining wheel as an aux button, but recommending a code comment describing which field is a wheel.

Comment on lines +147 to +151
> If you specify report lengths as null, it can be hard to fix any future
conflicts that may arise without knowing what values your specific tablet
has. If you do decide to specify them as null, please make sure to include a
OpenTabletDriver diagnostics with the pull request, or include the lengths in
the commit description.
Copy link
Member

Choose a reason for hiding this comment

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

Diagnostics should always be included as specified within the CONTRIBUTING.md, it can be extremely useful especially with the wider options with the information in 0.6.4.0 onwards.

Not sure anything really needs to be changed here but I thought I might mention it while I'm reading it.


[opentabletdriver.github.io#171]: https://github.com/OpenTabletDriver/opentabletdriver.github.io/issues/171

### Device strings
Copy link
Member

Choose a reason for hiding this comment

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

You haven't really mentioned how to get these device strings? For something as simple as this it would be nice to avoid the guesswork on a process that can have quite a bit of guesswork if you don't know what you are doing?

This is a bit of a mess (because i'm repiping unsorted knowledge directly from my brain without thinking about it first), and applies to initialization strings too but...

Inits for huion/gaomon (most common to use strings):

  • 200 is usually the only one required if its present (sometimes 123 is required for aux)
  • if 200 isn't present, its usually 100, accompanied sometimes by 123 for auxiliary key support (users should test without 123 first)

I'm not sure what to put for device string matching? maybe pull some examples from the huion and gaomon configs as you have different patterns for those and explain how to give them the most compatibility (regex for any 6 numbers instead of specific numbers etc etc) while not being verbose that it could match tablets that it shouldn't.

I might try and write something up and suggest it directly, trying to condense all of my personal experience down into a small document is... hard.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would be nice if you have more to add, because after having done support over the last few weeks I've observed that getting this section right is a lot more critical than ever.

Comment on lines +260 to +261
If any of the values seems absent or malfunctioning, you are probably
using the wrong parser.
Copy link
Member

Choose a reason for hiding this comment

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

Should we mention some symptoms of using the wrong parser?

Suggested change
If any of the values seems absent or malfunctioning, you are probably
using the wrong parser.
If any of the values seems absent or malfunctioning (overflowing, abrupt jumps),
you are probably using the wrong parser.

Also crashing is a symptom too, but this could also be a symptom of using the wrong interface and OTD trying to parse garbage makes it crash.

In cases where you are trying to write/figure out what parser to use and you keep getting crashes, recommending the PassthroughReportParser could be good.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can mention crashing - do you have any specific exceptions in mind?

Thanks for mentioning PassthroughReportParser, I'll get that added in some capacity.


A barebones tablet report that only reports an `ITabletReport`

### Init strings (Feature/OutputInitReport, InitializationStrings)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we really have added anything to this doc to indicate having the right initialization process?

Maybe add something somewhere (before deciding for a parser) that you should be able to see raw data in the debugger change when you use the tablet and that they'll be no failed to init messages in the debugger. (provide examples from feature/output failure? init strings do not have a message, so an example can't be provided)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, makes sense. Will change it to suggest starting out with PassthroughReportParser and then getting init strings right before deciding on the exact parser.

Comment on lines +267 to +268
Verify that the X and Y values increment monotonically. Critically, the value
must never wrap around.
Copy link
Member

Choose a reason for hiding this comment

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

Noticed that you mentioned that it shouldn't overflow here, but I'll leave my other comment as other pieces of information in there are useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what you think needs to be changed here?

Comment on lines +388 to +389
If anything is not working to your expectations, you might be using the wrong
parser, or you specified some values incorrectly.
Copy link
Member

Choose a reason for hiding this comment

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

Could we mention the Discord server directly here? If anybody is discouraged because they can't get it working a direct link to help will connect to them more than just the header at the top of the screen.

@gonX gonX self-assigned this Jan 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wiki Improvements or additions to the wiki
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants