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

Dual analog support #380

Merged
merged 9 commits into from
Jul 5, 2023
Merged

Conversation

arntsonl
Copy link
Contributor

Dual analog support, added selection for left/right analog (maybe reetrict this?), and added back in dead zone and forced circularity to web config.

…trict this?), and added back in dead zone and forced circularity to web config.
@TheTrainGoes
Copy link
Contributor

There are a few odd things with this in my testing:

1 - After setting the pins via web-config you cannot change them to anything else. You need to set them to none, then save, then put the new pins in.
2 - There is no way to invert or flip the signals (means board makers will need to be very careful in how they orient the part
3 - I was getting a lot of dead zones on my test unit but that may just be poor quality analog sticks

@mthiesen
Copy link
Contributor

I am wondering why you keep analogAdcPinX and analogAdcPinY around when you no longer use them. You could probably keep using them instead of the newly added analogAdc1PinX and analogAdc1PinY. If you want to do this, you can simply rename the properties in config.proto. Protobuf does not care about the actual names, only the IDs. As long as the IDs and types stay the same, you can safely rename the properties.

If for some reason you cannot reuse these two properties, you should remove them from config.proto. The best way to do this is to comment out the lines so that we do not accidentally reuse the IDs later.

@arntsonl
Copy link
Contributor Author

arntsonl commented Jul 4, 2023

@mthiesen I was going to ask actually, okay so the correct way is to rename the properties and keep the IDs the same?

So this would be correct?

message AnalogOptions
{
	optional bool enabled = 1;
	optional int32 analogAdc1PinX = 2; // previously analogAdcPinX
	optional int32 analogAdc1PinY = 3; // previously analogAdcPinY
	optional bool forced_circularity = 4;
	optional uint32 analog_deadzone = 5;
	optional int32 analogAdc2PinX = 6;
	optional int32 analogAdc2PinY = 7;
	optional DpadMode analogAdc1Mode = 8;
	optional DpadMode analogAdc2Mode = 9;
}

Let me know and I'll make that change!

@mthiesen
Copy link
Contributor

mthiesen commented Jul 4, 2023

@arntsonl Yes this is precisely what I meant.

@arntsonl arntsonl marked this pull request as ready for review July 4, 2023 20:20
@arntsonl
Copy link
Contributor Author

arntsonl commented Jul 4, 2023

Ready for review!

Copy link
Member

@SavageCore SavageCore left a comment

Choose a reason for hiding this comment

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

Some more fixes to come, merging this in to stop the endless rebasing

@SavageCore SavageCore merged commit 521f642 into OpenStickCommunity:main Jul 5, 2023
@arntsonl arntsonl deleted the dual_analog branch August 18, 2023 19:12
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.

4 participants