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

OpenTX 2.1 killEvents not working? #2469

Closed
pinkywafer opened this issue Jul 15, 2015 · 17 comments
Closed

OpenTX 2.1 killEvents not working? #2469

pinkywafer opened this issue Jul 15, 2015 · 17 comments

Comments

@pinkywafer
Copy link

A simple test Telemetry script:

local function run(event)
  lcd.clear()
  lcd.drawScreenTitle(" Test",1,1)
  killEvents(event)
end

return { run=run }

should display the screen title, and not respond to any key presses (as killEvents should catch and clear them), but all keys function normally.

@projectkk2glider
Copy link
Member

I think using killEvents is not meant to be used in the Telemetry scripts. What would you use it for?

@pinkywafer
Copy link
Author

The main use I see is multi-page telemetry scripts - using button presses to change pages.

There has also been a telemetry script which had several pages and options to send stick commands to cleanflight: http://rcsettings.com/index.php/viewdownload/13-lua-scripts/204-cleanflight-commands

@projectkk2glider
Copy link
Member

Most of the keys are already used by the radio only MENU is unused. @bsongis and @kilrah what do you think, should we allow LUA scripts to take over the keys in Telemetry view? I could be dangerous.

@kilrah
Copy link
Member

kilrah commented Jul 18, 2015

For me all keys except EXIT and PAGE could be available to scripts. PAGE changes telemetry screens and we wouldn't want to block that, EXIT should always go back to the main views but other keys are useless from a system point of view in this context. ENTER LONG is used for the Reset menu, that could maybe stay for system use only as it might make sense also when using Lua telemetry screens.

@pinkywafer
Copy link
Author

Thanks very discussing this guys.
At the moment, however + and - keys are switching telemetry screens as well as page and page long

@pinkywafer
Copy link
Author

Just a thought... I can see why you say to prevent EXIT and ENTER LONG being used in killEvents, but is the PAGE key important? If not, you could then make a telemetry script with X pages, maintain the use of page to move through the screens until the last screen, then move to the next "native" telemetry page by not using killEvents at this point. ie:

if event == EVT_PAGE_BREAK then
    if currentPage +1 < numberOfPages then
        killEvents(event)
        currentPage = currentPage +1
    else
        --not needed as without killEvents next "native page is displayed"
    end
elseif event == EVT_PAGE_LONG then
    if currentPage > 1 then
        killEvents(event)
        currentPage = currentPage -1
    else
        --not needed as without killEvents previous "native page is displayed"
    end
end

@kilrah
Copy link
Member

kilrah commented Jul 19, 2015

It might be frustrating to the user as they might want to switch to the next telemetry screen NOW without having to scroll all the way through yours first.

So I'd rather keep PAGE for system use and use MENU for navigation between multiple pages of a given script. It also doesn't change what has kind of become the de facto standard for that in 2.0.

@pinkywafer
Copy link
Author

OK, that makes sense.
if it's possible to use the + and - and ENTER (not long) too that would be nice!

@kilrah
Copy link
Member

kilrah commented Jul 19, 2015

Yes, as I said to me everything but PAGE and EXIT (short and long) and ENTER long could be available to scripts.

@bsongis
Copy link
Member

bsongis commented Jul 19, 2015

I would see only + - and ENTER (not long) available. It should work this way, no?

@dsbeach
Copy link
Contributor

dsbeach commented Nov 10, 2015

I'm digging into this today - it's something I'd really like to solve for my own telemetry scripts that process keys (+ - ENT and EXIT).

It appears to be related to the deprecation of lcd.lock() in 2.1.

Here is a relevant section of 2.0 code from opentx.cpp in perMain(). If a Lua script locks the lcd the key event is not passed on to the menu function that advances the telemetry screen

    if (!LCD_LOCKED()) {
      lcd_clear();
      g_menuStack[g_menuStackPtr]((warn || menu) ? 0 : evt);
    }

Under 2.1 in main_arm.cpp function perMain() the menu function is invoked regardless of the key state (around line 185)

    g_menuStack[g_menuStackPtr]((warn || menu) ? 0 : evt);

So... What is the best approach to a fix? What was the logic behind deprecating lcd.lock()? Should it be implemented now, or should there be code added to correctly process the killed() events?

@dsbeach
Copy link
Contributor

dsbeach commented Nov 10, 2015

Testing the above pull request looks good so far.

Below is a Lua script to try. It appears that this fix enables all keys to be processed in a script except for:

  • EXIT - not seen by the script and immediately exits telemetry screen display mode
  • repeat keys (EVT_PLUS_RPT and EVT_MINUS_RPT) are not seen

I like the idea of allowing Lua scripts to process the page key events. I think it's OK for a script developer to create a multi page telemetry script as long as it behaves in a predictable manner.

local lastKey = 'none'
  return {
    run = function(event)
      if event == EVT_EXIT_BREAK then
        lastKey = "EVT_EXIT_BREAK"
        killEvents(event)
      elseif event == EVT_MENU_BREAK then
        lastKey = "EVT_MENU_BREAK"
        killEvents(event)
      elseif event == EVT_PAGE_BREAK then
        lastKey = "EVT_PAGE_BREAK"
        -- killEvents(event)
      elseif event == EVT_PAGE_LONG then
        lastKey = "EVT_PAGE_LONG"
        -- killEvents(event)
      elseif event == EVT_ENTER_BREAK then
        lastKey = "EVT_ENTER_BREAK"
        killEvents(event)
      elseif event == EVT_ENTER_LONG then
        lastKey = "EVT_ENTER_LONG"
        killEvents(event)
      elseif event == EVT_PLUS_BREAK then
        lastKey = "EVT_PLUS_BREAK"
        killEvents(event)
      elseif event == EVT_MINUS_BREAK then
        lastKey = "EVT_MINUS_BREAK"
        killEvents(event)
      elseif event == EVT_PLUS_RPT then
        lastKey = "EVT_PLUS_RPT"
        killEvents(event)
      elseif event == EVT_MINUS_RPT then
        lastKey = "EVT_MINUS_RPT"
        killEvents(event)
      elseif event == EVT_PLUS_FIRST then
        lastKey = "EVT_PLUS_FIRST"
        killEvents(event)
      elseif event == EVT_MINUS_FIRST then
        lastKey = "EVT_MINUS_FIRST"
        killEvents(event)
      end
      lcd.clear()
      lcd.drawText(10, 20, "idx : ", 0)
      lcd.drawText(lcd.getLastPos(), 20, lastKey, 0)
    end
  }

projectkk2glider added a commit that referenced this issue Nov 20, 2015
…metry screen, EXIT LONG added as an exit from the telemetry screen
projectkk2glider added a commit that referenced this issue Nov 21, 2015
…metry screen, EXIT LONG added as an exit from the telemetry screen
bsongis added a commit that referenced this issue Nov 24, 2015
…keys

Fixes #2469: Allow usage of PLUS, MINUS, MENU and ENT keys in telemet…
@projectkk2glider
Copy link
Member

Reopening until I make cosmetic changes and port this to the next branch

@projectkk2glider projectkk2glider self-assigned this Nov 25, 2015
bsongis added a commit that referenced this issue Nov 26, 2015
…keys_for_next_part1

Re #2469: part1: Menu system renaming of variables and refactoring
projectkk2glider added a commit that referenced this issue Nov 26, 2015
bsongis added a commit that referenced this issue Nov 26, 2015
…keys_for_next_part2

Re #2469: part2: keys accesible to Lua telemetry scripts (Taranis only)
@projectkk2glider
Copy link
Member

Reopening till the Horus main gets fixed. The commit 806bd1d did not really solve this, in fact it is not even in the main branch. Github got confused somehow.

@dsbeach
Copy link
Contributor

dsbeach commented Dec 13, 2015

Wasn't this fixed by #3112 ? Main is working, and I'll test next later today.

@projectkk2glider
Copy link
Member

Wasn't this fixed by #3112 ? Main is working, and I'll test next later today.

It was for Taranis, the Horus uses different GUI logic and I haven’t managed to port the changes yet.

@projectkk2glider
Copy link
Member

projectkk2glider commented Oct 3, 2016

Closing, Horus solution in #3821.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants