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.

Sign up for GitHub

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

Jump to bottom

Add handling of button release events for Octavi devices #1627

Merged
merged 7 commits into from
Feb 10, 2024
Merged

Add handling of button release events for Octavi devices #1627

merged 7 commits into from
Feb 10, 2024

Conversation

tfinnie
Copy link
Contributor

@tfinnie tfinnie commented Jan 27, 2024

Fixes Issue #1626

neilenns
neilenns suggested changes Jan 27, 2024
View reviewed changes
MobiFlight/Joysticks/Octavi/Octavi.cs Outdated Show resolved Hide resolved
MobiFlight/Joysticks/Octavi/Octavi.cs Outdated Show resolved Hide resolved
@tfinnie
Copy link
Contributor Author

tfinnie commented Jan 28, 2024

While working on this code I've noticed some other behaviour which doesn't seem correct to me:

  1. The AP context is toggling shift mode when pressing the encoder button and I don't think it should be. This is giving rise to the following button events: Button_AP^_TOG, Button_AP^_OI, Button_AP^_OD, Button_AP^_II, Button_AP^_ID. Can these be removed and the shift mode removed also? It is also causing the Button_AP_CRSR event to only fire every second time the encoder button is pressed.
  2. COM2, NAV1, NAV2, and XPDR are raising Button_COM2_CRSR, Button_NAV1_CRSR, Button_NAV2_CRSR and Button_XPDR_CRSR events respectively (COM1 is not doing this). I don't think these should exist since pushing the encoder button is for toggling shift mode in these contexts.

tfinnie
tfinnie commented Jan 31, 2024
View reviewed changes
MobiFlight/Joysticks/Octavi/OctaviHandler.cs Outdated
ButtonPress = OctaviButtonMatrix.ElementAt(ButtonPress).Value; // translate to existing Octavi "devices" in MF
if (ButtonPress >= 0) ButtonPresses.Add(ButtonPress); // if not "unassigned" (-1), then press the button!
int lookupIndex = extendedContextState * 16 + HidEventAssignments[button]; // find "full matrix" event index
int buttonIndex = OctaviButtonMatrix.ElementAt(lookupIndex).Value; // translate to existing Octavi "devices" in MF
Copy link
Contributor Author

Choose a reason for hiding this comment

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

OctaviButtonMatrix is a Dictionary which has no guaranteed order. Using ElementAt is not really the correct way to do this.

@tfinnie
Copy link
Contributor Author

tfinnie commented Feb 2, 2024

I would also like to suggest a renaming of the AP buttons like so, to better represent their actual functions and the Octavi hardware:

  • Button_AP_CDI -> Button_AP_AP
  • Button_AP_OBS -> Button_AP_HDG
  • Button_AP_MSG -> Button_AP_NAV
  • Button_AP_FPL -> Button_AP_APR
  • Button_AP_VNAV -> Button_AP_ALT
  • Button_AP_PROC -> Button_AP_VS

This would obviously break exisiting configuration though. Not sure how big of a deal that is?

@DocMoebiuz
Copy link
Collaborator

don't mix any renaming and the initial scope of release events

Felix has to chime in on the renaming so make a separate PR for that

@tfinnie tfinnie mentioned this pull request Feb 2, 2024
Closed
@tfinnie
Copy link
Contributor Author

tfinnie commented Feb 2, 2024

I've created Issue #1642 for this. I think I should hold off on creating a PR until this one is finalised otherwise it will create merge issues.

tfinnie and others added 7 commits February 3, 2024 08:48
@tfinnie
added handling of button release events for Octavi devices
6c78865
@tfinnie @neilenns
Use var instead of explicit type and variable renamed as per PR sugge…
6e252d5 
…stion

Co-authored-by: Neil Enns <neile@live.com>
@tfinnie @neilenns
Variable renamings as per PR suggestion
2df95c1 
Co-authored-by: Neil Enns <neile@live.com>
@tfinnie
Code cleanup: indentation fixes, removed unused and redundant code, r…
d3c64bd 
…enamed local variables to correct casing
@tfinnie
Additional code cleanup and variable renaming
497c61c
@tfinnie
Converted OctaviButton enum to a Flags enum and replaced use of uint …
bcba4ae 
…etc for enums to give stronger typing and clearer code.
@tfinnie
Big refactoring and removal of erroneous buttons "Button_COM2_CRSR", …
63ac017 
…"Button_NAV1_CRSR", "Button_NAV2_CRSR", "Button_XPDR_CRSR ". Also removal of Shift mode for AP context.
DocMoebiuz
DocMoebiuz approved these changes Feb 10, 2024
View reviewed changes
Copy link
Collaborator

@DocMoebiuz DocMoebiuz left a comment

Choose a reason for hiding this comment

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

Thanks. Looks a lot cleaner now.

MobiFlight/Joysticks/Octavi/OctaviHandler.cs
foreach(uint HIDEvent in HIDEventAssignments.Keys)

// Encoders
// TODO: Should we add RELEASE events for the encoders too?
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are no release events for encoders.

@github-actions GitHub Actions
Copy link

Build for this pull request:
MobiFlightConnector.zip

@DocMoebiuz DocMoebiuz merged commit 912ad05 into MobiFlight:main Feb 10, 2024
1 check passed
@DocMoebiuz DocMoebiuz added enhancement Improvement of an existing feature firmware and removed enhancement Improvement of an existing feature labels Feb 10, 2024
@DocMoebiuz DocMoebiuz mentioned this pull request Feb 11, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neilenns neilenns neilenns requested changes

@DocMoebiuz DocMoebiuz DocMoebiuz approved these changes

Assignees
No one assigned
Labels
firmware
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tfinnie @DocMoebiuz @neilenns