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

Joystick Issues #799

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/libs/joystick/manager.ts
Expand Up @@ -32,6 +32,7 @@ const JoystickMapVidPid: Map<string, JoystickModel> = new Map([
['054c:0ce6', JoystickModel.DualSense],
['054c:09cc', JoystickModel.DualShock4],
['045e:02e0', JoystickModel.XboxOneS_Bluetooth],
['045e:02fd', JoystickModel.XboxOneS_Bluetooth],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rafaellehmkuhl I have a xbox one s controller but it showed up different on my machine. (ubuntu 20.22)

Copy link
Member

Choose a reason for hiding this comment

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

From what I'm checking here we are actually using the "Xbox One Controller Bluetooth" PID as an "S" version here, which it isn't.

Could you keep your change and also change the XboxOneS_Bluetooth to XboxOne_Bluetooth?

I don't have this controller to confirm. If I remember correctly @patrickelectric is the one who has it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rafaellehmkuhl I checked the website and my controller being "Xbox One S Controller [Bluetooth]" is correct. I'll be interested to see what you all have.

Copy link
Member

Choose a reason for hiding this comment

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

Yup, I'm agreeing with you haha

I think one of us has an Xbox One controller, thought it was an S one, but was actually a regular One, and we added it incorrectly.

['045e:0b13', JoystickModel.XboxController_Bluetooth],
['045e:0b12', JoystickModel.XboxController_Wired],
['28de:11ff', JoystickModel.XboxController_360],
Expand Down Expand Up @@ -272,10 +273,13 @@ class JoystickManager {
*/
getModel(gamepad: Gamepad): JoystickModel {
const { vendor_id, product_id } = this.getVidPid(gamepad)
console.log(vendor_id, product_id)

if (vendor_id == undefined || product_id == undefined) {
return JoystickModel.Unknown
}
console.log('joystick pid')
console.log(JoystickMapVidPid.get(`${vendor_id}:${product_id}`))
return JoystickMapVidPid.get(`${vendor_id}:${product_id}`) ?? JoystickModel.Unknown
}

Expand Down
38 changes: 27 additions & 11 deletions src/libs/joystick/protocols/mavlink-manual-control.ts
Expand Up @@ -379,13 +379,17 @@ export class MavlinkManualControlManager {
if (!this.joystickState || !this.currentActionsMapping || !this.activeButtonsActions) return

// Return if insuficient vehicle data (prevent dangerous manual control messages)
if (!this.currentVehicleParameters || !this.vehicleButtonParameterTable) return
if (!this.currentVehicleParameters || !this.vehicleButtonParameterTable)
{
console.log("insufficient vehicle data")
return
}

// Instantiate manual control state if not already done
if (!this.manualControlState) {
this.manualControlState = new MavlinkManualControlState()
}

console.log("manual control update")
const buttonParametersNamedObject: { [key in number]: string } = {}
this.vehicleButtonParameterTable.forEach((entry) => (buttonParametersNamedObject[entry.value] = entry.title))
const currentRegularButtonParameters = Object.entries(this.currentVehicleParameters)
Expand All @@ -395,6 +399,9 @@ export class MavlinkManualControlManager {
.filter(([k]) => k.includes('BTN') && k.includes('S'))
.map((btn) => ({ button: btn[0], actionId: buttonParametersNamedObject[btn[1]] }))


console.log(currentRegularButtonParameters)

const activeMavlinkManualControlActions = this.activeButtonsActions.filter((a) => a.protocol === JoystickProtocol.MAVLinkManualControl).map((action) => action.id)
const regularVehicleButtonsToActivate = currentRegularButtonParameters
.filter((entry) => activeMavlinkManualControlActions.includes(entry.actionId as MAVLinkButtonFunction))
Expand All @@ -403,6 +410,8 @@ export class MavlinkManualControlManager {
.filter((entry) => activeMavlinkManualControlActions.includes(entry.actionId as MAVLinkButtonFunction))
.map((entry) => manualControlButtonFromParameterName(entry.button))

console.log(regularVehicleButtonsToActivate)

const useShift = this.activeButtonsActions.map((a) => a.id).includes(modifierKeyActions.shift.id)
const shiftButton = currentRegularButtonParameters.find((v) => v.actionId === MAVLinkButtonFunction.shift)

Expand Down Expand Up @@ -502,16 +511,23 @@ export class MavlinkManualControlManager {
// Re-use shift button if already mapped. Otherwise use R0 and S0.
const regularShiftFunction = currentRegularButtonParameters.find((v) => v.actionId === MAVLinkButtonFunction.shift)
const shiftActionValue = this.vehicleButtonParameterTable.find((e) => e.title === MAVLinkButtonFunction.shift)
if (regularShiftFunction === undefined) {
// Map shift to R0
this.vehicle.setParameter({ id: MAVLinkManualControlButton.R0, value: shiftActionValue!.value })

// Map shift to S0
this.vehicle.setParameter({ id: MAVLinkManualControlButton.S0, value: shiftActionValue!.value })
} else {
const sFunction = regularShiftFunction.button.replace('FUNCTION', 'SFUNCTION')
this.vehicle.setParameter({ id: sFunction, value: shiftActionValue!.value })
// use try catch to avoid undefined error

try {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rafaellehmkuhl without this try catch, the joystick module kept crashing on me.

Copy link
Member

Choose a reason for hiding this comment

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

What error is happening? I think it would be good if we treat it directly instead of putting this entire code on a try/catch. We can be sweeping problems under the rug here.

As we didn't have complaints about it till now, I would bet they have something to do with the vehicle type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR isn't ment to go in, but mostly to review the changes I needed to make to get my setup working, this way we can come up with more permanent fixes together. I'll have to get you the exact error I was getting later, but I believe the module was crashing on the setParameter call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rafaellehmkuhl This is the error I get some times with my controllers. I still have not figured out exactly what causes it.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

without the try catch the controller stops working all together

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After further debugging it looks like this error is only present when I use my F310 joystick which I have attempted to add support for on my local copy. Still not sure what I did wrong with my F310 implementation, but I created PRs for my features that are independent of my F310 support

if (regularShiftFunction === undefined) {
// Map shift to R0
this.vehicle.setParameter({ id: MAVLinkManualControlButton.R0, value: shiftActionValue!.value })

// Map shift to S0
this.vehicle.setParameter({ id: MAVLinkManualControlButton.S0, value: shiftActionValue!.value })
} else {
const sFunction = regularShiftFunction.button.replace('FUNCTION', 'SFUNCTION')
this.vehicle.setParameter({ id: sFunction, value: shiftActionValue!.value })
}
} catch (e) {
console.error(e)
}


const currentMappedActionsInRegularButtons = currentRegularButtonParameters.map((v) => v.actionId)
const currentMappedActionsInShiftButtons = currentShiftButtonParameters.map((v) => v.actionId)
Expand Down