Some hotkeys are not working on today's master #4149

Hakansv · 2024-09-24T16:51:19Z

Describe the bug
Today's master. Several hotkeys are not working
To Reproduce
Steps to reproduce the behavior:

Start O
Try any hot key. W, Ctrl M, Ctrl R, M - no response
Functional keys are though working like F4, F11
The new "?" is working. :)

Desktop (please complete the following information if applicable):

OS: Win10

leamas · 2024-09-24T19:32:49Z

Probably related to 48cabbc. However, the fix is no t to revert this but to handle it correct.

nohal · 2024-09-24T19:37:01Z

Not broken on Windows only, the same problem is visible on Linux as well.
And I have a vague feeling from very distant past that EVT_CHAR is unusable for our purpose and that is why the hotkey handling was done the way it was done.

leamas · 2024-09-24T21:28:32Z

Sloppy coding from my side anyway. Patch below should fix it, but I need some sleep now....

diff --git a/gui/src/chcanv.cpp b/gui/src/chcanv.cpp
index 4a5ab6e63..5500e6847 100644
--- a/gui/src/chcanv.cpp
    b/gui/src/chcanv.cpp
@@ -370,7  370,7 @@ EVT_TIMER(CURTRACK_TIMER, ChartCanvas::OnCursorTrackTimerEvent)
 EVT_TIMER(ROT_TIMER, ChartCanvas::RotateTimerEvent)
 EVT_TIMER(ROPOPUP_TIMER, ChartCanvas::OnRolloverPopupTimerEvent)
 EVT_TIMER(ROUTEFINISH_TIMER, ChartCanvas::OnRouteFinishTimerEvent)
-EVT_CHAR(ChartCanvas::OnKeyDown)
 EVT_KEY_DOWN(ChartCanvas::OnKeyDown)
 EVT_KEY_UP(ChartCanvas::OnKeyUp)
 EVT_CHAR(ChartCanvas::OnKeyChar)
 EVT_MOUSE_CAPTURE_LOST(ChartCanvas::LostMouseCapture)
@@ -2591,8  2591,11 @@ void ChartCanvas::OnKeyChar(wxKeyEvent &event) {
                // anything else
 
   int key_char = event.GetKeyCode();
-
-  if (g_benable_rotate) {
   if (key_char == static_cast<int>('?')) {
     auto parent = wxWindow::FindWindowByName("MainWindow");
     HotkeysDlg dlg(parent);
     dlg.ShowModal();
   } else if (g_benable_rotate) {
     switch (key_char) {
       case ']':
         RotateCanvas(1);
@@ -3037,12  3040,6 @@ void ChartCanvas::OnKeyDown(wxKeyEvent &event) {
 
           break;
 
-        case '?': {
-          auto parent = wxWindow::FindWindowByName("MainWindow");
-          HotkeysDlg dlg(parent);
-          dlg.ShowModal();
-        } break;
-
         case 1:  // Ctrl A
           TogglebFollow();

leamas · 2024-09-24T21:37:58Z

BTW: This looks like something Very Undocumented? Any ideas what this is about?

} else if (g_benable_rotate) {
    switch (key_char) {
      case ']':
        RotateCanvas(1);
        Refresh();
        break;

      case '[':
        RotateCanvas(-1);
        Refresh();
        break;

      case '\\':
        DoRotateCanvas(0);
        break;
    }
  }

nohal · 2024-09-24T22:33:37Z

Hotkeys for canvas rotation, what is the problem with it?

leamas · 2024-09-25T05:03:26Z

None, besides that that are not documented.

nohal · 2024-09-25T05:29:14Z

They are documented in https://opencpn.org/wiki/dokuwiki/doku.php?id=opencpn:manual_basic:ui_user_interface:keyboard_shortcuts

Hakansv · 2024-09-25T07:04:47Z

@leamas

Sloppy coding from my side anyway. Patch below should fix it,

I tested the above patch:
The previously working hotkeys seems restored and functional. (Only tested on Win with qwerty-board)
The new "?"-key would though need more work. Now when I use "?" the chart is centered to the cursor location, like a left click, and the zoom level is changed. Then the hot key help windows is opened.

This part of the code seems tricky with a lot adaptations to our various systems and also tweaks for non-qwerty-keyboards.
Any change here may need tests on all system before merge?

leamas · 2024-09-25T08:39:02Z

Well, as long as you are ready to test a patch...

Hakansv · 2024-09-25T08:58:05Z

Well, as long as you are ready to test a patch...

I can of course test mainly on Win when available and happens to see a change is made. But I've no Mac and no other Linux than Debian and noting else than the qwerty keyboard.
I mean that it is up to the person who changes to perform or initiate the first tests?

leamas · 2024-09-25T09:20:10Z

This is actually not so much about platforms as keyboards. Below is an updated patch which does the right thing on Linux. No reason it will behave differently on windows. However, somewhat unsure what might happen on a sufficiently odd keyboard.

diff --git a/gui/src/chcanv.cpp b/gui/src/chcanv.cpp
index aa31dc9fa..102812b5f 100644
--- a/gui/src/chcanv.cpp
    b/gui/src/chcanv.cpp
@@ -370,7  370,7 @@ EVT_TIMER(CURTRACK_TIMER, ChartCanvas::OnCursorTrackTimerEvent)
 EVT_TIMER(ROT_TIMER, ChartCanvas::RotateTimerEvent)
 EVT_TIMER(ROPOPUP_TIMER, ChartCanvas::OnRolloverPopupTimerEvent)
 EVT_TIMER(ROUTEFINISH_TIMER, ChartCanvas::OnRouteFinishTimerEvent)
-EVT_CHAR(ChartCanvas::OnKeyDown)
 EVT_KEY_DOWN(ChartCanvas::OnKeyDown)
 EVT_KEY_UP(ChartCanvas::OnKeyUp)
 EVT_CHAR(ChartCanvas::OnKeyChar)
 EVT_MOUSE_CAPTURE_LOST(ChartCanvas::LostMouseCapture)
@@ -2591,7  2591,21 @@ void ChartCanvas::OnKeyChar(wxKeyEvent &event) {
                // anything else
 
   int key_char = event.GetKeyCode();
-
   switch (key_char) {
     case  static_cast<int>('?'): {
       auto parent = wxWindow::FindWindowByName("MainWindow");
       HotkeysDlg dlg(parent);
       dlg.ShowModal();
     } break;
     case ' ':
       ZoomCanvas(g_plus_minus_zoom_factor, false);
       break;
     case '-':
       ZoomCanvas(1.0 / g_plus_minus_zoom_factor, false);
       break;
     default:
       break;
   }
   if (g_benable_rotate) {
     switch (key_char) {
       case ']':
@@ -2856,18  2870,6 @@ void ChartCanvas::OnKeyDown(wxKeyEvent &event) {
     //      Handle both QWERTY and AZERTY keyboard separately for a few control
     //      codes
     if (!g_b_assume_azerty) {
-      switch (key_char) {
-        case ' ':
-        case '=':
-          ZoomCanvas(g_plus_minus_zoom_factor, false);
-          break;
-
-        case '-':
-        case '_':
-          ZoomCanvas(1.0 / g_plus_minus_zoom_factor, false);
-          break;
-      }
-
 #ifdef __WXMAC__
       if (g_benable_rotate) {
         switch (key_char) {
@@ -3037,12  3039,6 @@ void ChartCanvas::OnKeyDown(wxKeyEvent &event) {
 
           break;
 
-        case '?': {
-          auto parent = wxWindow::FindWindowByName("MainWindow");
-          HotkeysDlg dlg(parent);
-          dlg.ShowModal();
-        } break;
-
         case 1:  // Ctrl A
           TogglebFollow();

Hakansv · 2024-09-25T09:54:56Z

Yes, the above patch seems to work on Win. Also the "?" key wo disturbing chart center or zoom. Good!

The Mac function Pavel may test?
Who has a azerty keyboard, I don't know. Isn't that a French gadget?

leamas · 2024-09-25T10:06:15Z

I think I'll merge it as it is now. The important is that we don't break anything. We are still in an early stage, and that a feature is less than perfectly implemented should no be a reason to not merge. There is still time.

And then again, when I look at this again it actually seems like the "new" code should behave better than the old.

For the record, the problem faced was that pressing the Shift- key on a Nordic keyboard generates both a KeyDown event(' ') and a KeyChar event('?'). What I did was just to move the code which was triggered by KeyDown to KeyChar, thus only triggering the code when actually pressing a non-shifted ' ' key.

leamas · 2024-09-25T10:08:47Z

I think I'll merge it as it is now.

Done

rgleason · 2024-09-25T11:17:01Z

I note that the "About" icon in toolbar uses "?" Where Help is located, so is it intentional to use a keyboard shortcut "?" to get to the manual? Is this consistant enough?

Hakansv · 2024-09-25T11:26:28Z

Now after the latest merge all seems fine on Win.
@nohal Would you check on Mac and close this issue if OK?

bcn2 · 2024-09-26T09:53:35Z

On macOS 14.6.1 (kbd Spanish) the usual short cuts do work.
However the "?" key does no do anything.
Version ...17C7C

leamas · 2024-09-26T14:05:24Z

Probably something about the keyboard. Could you please try patch below, press the '?' key and report back?'

diff --git a/gui/src/chcanv.cpp b/gui/src/chcanv.cpp
index cc809bca6..2d0f25b30 100644
--- a/gui/src/chcanv.cpp
    b/gui/src/chcanv.cpp
@@ -2591,6  2591,7 @@ void ChartCanvas::OnKeyChar(wxKeyEvent &event) {
                // anything else
 
   int key_char = event.GetKeyCode();
 std::cout << "keychar: " << static_cast<char>(key_char) << "\n";
   switch (key_char) {
     case '?':
       HotkeysDlg(wxWindow::FindWindowByName("MainWindow")).ShowModal();

bcn2 · 2024-09-26T16:26:22Z

Sorry, but no way for me to build on that environment......

leamas · 2024-09-26T16:36:14Z

OK. But out of curiosity: How did you test 2b0017c if you were not able to build?

nohal · 2024-09-26T16:39:22Z

OK. But out of curiosity: How did you test 2b0017c if you were not able to build?

Using the package from https://cloudsmith.io/~david-register/repos/opencpn-unstable/packages/

I will handle this after I unpack the box with the Mac in the new house...

Hakansv added the bug label Sep 24, 2024

Hakansv changed the title ~~Win: No hotkeys are working on today's master~~ Win: Some hotkeys are not working on today's master Sep 24, 2024

nohal changed the title ~~Win: Some hotkeys are not working on today's master~~ Some hotkeys are not working on today's master Sep 24, 2024

leamas self-assigned this Sep 24, 2024

Hakansv closed this as completed Sep 26, 2024

Hakansv reopened this Sep 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some hotkeys are not working on today's master #4149

Some hotkeys are not working on today's master #4149

Hakansv commented Sep 24, 2024 •

edited

Loading

leamas commented Sep 24, 2024

nohal commented Sep 24, 2024

leamas commented Sep 24, 2024 •

edited

Loading

leamas commented Sep 24, 2024

nohal commented Sep 24, 2024

leamas commented Sep 25, 2024

nohal commented Sep 25, 2024

Hakansv commented Sep 25, 2024

leamas commented Sep 25, 2024

Hakansv commented Sep 25, 2024

leamas commented Sep 25, 2024

Hakansv commented Sep 25, 2024

leamas commented Sep 25, 2024

leamas commented Sep 25, 2024

rgleason commented Sep 25, 2024

Hakansv commented Sep 25, 2024

bcn2 commented Sep 26, 2024 •

edited

Loading

leamas commented Sep 26, 2024

bcn2 commented Sep 26, 2024

leamas commented Sep 26, 2024

nohal commented Sep 26, 2024

Some hotkeys are not working on today's master #4149

Some hotkeys are not working on today's master #4149

Comments

Hakansv commented Sep 24, 2024 • edited Loading

leamas commented Sep 24, 2024

nohal commented Sep 24, 2024

leamas commented Sep 24, 2024 • edited Loading

leamas commented Sep 24, 2024

nohal commented Sep 24, 2024

leamas commented Sep 25, 2024

nohal commented Sep 25, 2024

Hakansv commented Sep 25, 2024

leamas commented Sep 25, 2024

Hakansv commented Sep 25, 2024

leamas commented Sep 25, 2024

Hakansv commented Sep 25, 2024

leamas commented Sep 25, 2024

leamas commented Sep 25, 2024

rgleason commented Sep 25, 2024

Hakansv commented Sep 25, 2024

bcn2 commented Sep 26, 2024 • edited Loading

leamas commented Sep 26, 2024

bcn2 commented Sep 26, 2024

leamas commented Sep 26, 2024

nohal commented Sep 26, 2024

Hakansv commented Sep 24, 2024 •

edited

Loading

leamas commented Sep 24, 2024 •

edited

Loading

bcn2 commented Sep 26, 2024 •

edited

Loading