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

Rework joystick calibration (2) #4470

Merged
merged 7 commits into from Feb 6, 2017

Conversation

jaxxzer
Copy link
Collaborator

@jaxxzer jaxxzer commented Jan 25, 2017

This is functionally equivalent to #4463, implemented differently according to Don's suggestion. The mode selection behavior in joystickConfigController now matches that of RC calibration.

screenshot 2017-01-24 23 33 31
screenshot 2017-01-24 23 33 44

The major difference is that selecting a mode just remaps the axes in _rgFunctionAxis, and the output logic and saved settings remain unchanged. The saved settings for the function->axis mappings are always saved as TX Mode 2 (current settings) mappings. A few settings are added for the saved joystick TX mode while running different airframes. The settings are forward/backward compatible tested on 3.1.1.

The calibration prompts haven't been changed in this one, and still ask to move throttle/pitch sticks, according to the selected mode.


}

void Joystick::setTXMode(int mode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this to handle a mode change in the middle of a calibration? RC Cal doesn't really handle that correctly not that I think of it. If so, I wonder if it would be simpler to just disable the mode change radios while in the middle of a cal?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is to change the mode mapping after calibration has been performed. I disabled the mode mapping selection for joysticks during calibration. I noticed it stays enabled for RC calibration, but that should probably be disabled, too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The TX mode can be changed before or after calibration, but not during. The TX mode is saved in settings, but the function axis settings are not affected. They are adjusted after the settings are loaded to match the TX mode setting, and they are adjusted again before saving. The function axis settings always reflect a mode 2 configuration (as they always have).

Copy link
Collaborator Author

@jaxxzer jaxxzer Feb 1, 2017

Choose a reason for hiding this comment

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

To clarify,

The function axis settings always reflect a mode 2 configuration (as they always have).

The saved function axis settings always reflect a mode 2 configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

The TX mode can be changed before or after calibration

Ok, that's kind of what I thought. My concern is that this makes joystick cal different from rc cal which does not support this. It also add an extra level of complexity. Do you think this is important. I'd rather keep it simpler and just like RC cal. But if it's good functionality then ok.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this reflects a difference in joysticks and RC transmitters, RC transmitters may come built for a specific mode that is hard-wired. You are selecting the mode that corresponds to the RC transmitter in your hands. Joysticks aren't made this way, so you are selecting the mode that you want your joystick to emulate.

If we remove this functionality, then we won't be able to auto-calibrate gamecontrollers, we won't be able to have vehicle-specific settings, and we will have to recalibrate the joystick in order to change the mode. I don't know if it will be clear that you need to recalibrate joysticks in order to change the mode either. We could have a message somewhere that says you must recalibrate the joystick to change the mode. Or we could disable the mode selection at all times except, as the first step in calibration.

In my opinion it is 'good functionality', but I understand your concerns. It is not absolutely necessary to leave this in. I did structure the commits so it would be straightforward to take out or revert.

Copy link
Contributor

Choose a reason for hiding this comment

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

All good enough explanation for me

exclusiveGroup: modeGroup
text: "1"
checked: controller.transmitterMode == 1
enabled: !controller.calibrating
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mode mapping selection is disabled here during calibration.

@DonLakeFlyer
Copy link
Contributor

@jaxxzer Sorry but I'm a little scattered on whether there was anything left to do here. Is this all good to merge now from where you stand? I'm clear from my end.

@jaxxzer
Copy link
Collaborator Author

jaxxzer commented Feb 6, 2017

Forcing recalibration after this merge as an extra measure. This will handle case in which someone may have previously calibrated joystick and intentionally mapped sticks in a different way than the pictures presented during the calibration process in order to achieve their desired mappings.

Good to go for me.

@DonLakeFlyer
Copy link
Contributor

Ok. Thanks @jaxxzer

@DonLakeFlyer DonLakeFlyer merged commit cdde6b7 into mavlink:master Feb 6, 2017
@jaxxzer jaxxzer deleted the calibration branch February 14, 2017 21:39
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.

None yet

2 participants