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

Added logic in perMain() to suppress killed events. Fixes #2469 #3051

Closed
wants to merge 1 commit into from
Closed

Added logic in perMain() to suppress killed events. Fixes #2469 #3051

wants to merge 1 commit into from

Conversation

dsbeach
Copy link
Contributor

@dsbeach dsbeach commented Nov 10, 2015

Fixes #2469

The logic to bypass killed events looked pretty straight forward so I took a swing at it.

I'm still testing for unwanted side effects, but I'd like some more feedback on the proposed changes.

@projectkk2glider
Copy link
Member

Looks good. One idea comes to mind: we might not allow (or still pass on) killing of certain key presses, for example long pres on EXIT key (that allows exit from the script is something goes wrong).

@projectkk2glider projectkk2glider added this to the OpenTX 2.1.X milestone Nov 10, 2015
@dsbeach
Copy link
Contributor Author

dsbeach commented Nov 10, 2015

As I just noted in #2469 - with this fix a script will not see any EXIT or _RPT events. I'd like to be able to process EXIT, but I can live without it.

@dsbeach
Copy link
Contributor Author

dsbeach commented Nov 16, 2015

Is there more discussion needed on this? I'd like to see it in the next release.

@bsongis
Copy link
Member

bsongis commented Nov 16, 2015

[EXIT Long] on a script which calls killEvents won't stop the script, am I mistaken?

@dsbeach
Copy link
Contributor Author

dsbeach commented Nov 16, 2015

In my testing Lua telemetry scripts never received the [Exit] or [Exit Long] events, and therefor could not call killEvents for them.

I'll do some testing with a one-time script and see what effect this has.

Would it help if I created a opentx/dsbeach/issue2469 branch so you can more easily test?

@dsbeach
Copy link
Contributor Author

dsbeach commented Nov 16, 2015

With this modification and #3024 (needed to test, and waiting for pull) I tested the script below as both a one-time and telemetry script.

Results as one-time script:

  • [Exit] is detected and can be processed
  • [Exit Long] is NOT DETECTED by script and 'Script force exit' appears in debug log

Results as telemetry script:

  • Neither [Exit] nor [Exit Long] are detected by telemetry script. Display reverts to main menu.

Script source:

local lastKey = 'none'

local function showEvent(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"
        print("EVT_PAGE_BREAK - ** NOT KILLED **")
        -- allow default handling for page event
      elseif event == EVT_PAGE_LONG then
        lastKey = "EVT_PAGE_LONG"
        print("EVT_PAGE_LONG - ** NOT KILLED **")
        -- allow default handling for page long 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

local function run(event)
  if ((event == EVT_EXIT_BREAK) or (event == EVT_EXIT_LONG)) then
    print ("returning 1 - event: " .. event)
    return 1
  else
    showEvent(event)
    if (not (event == 0)) then
      print ("returning 0 - event: " .. event)
    end
    return 0
  end
end

return { run = run }

@projectkk2glider
Copy link
Member

Where did you get EVT_EXIT_LONG? It is not defined in Lua, so I guess nil is used for it. That explains why you don't see it.

You don’t see EVT_EXIT_BREAK, because the telemetry script gets killed when we get EVT_KEY_FIRST(KEY_EXIT), but that event also gets passed to the script first!

So in effect you don't see these event, because the main code reacts faster, but the rogue Lua script (using custom event constants) could catch all EXIT events and for example prevent generation of EXIT LONG (I think).

@dsbeach
Copy link
Contributor Author

dsbeach commented Nov 22, 2015

Closing in light of #3087

@dsbeach dsbeach closed this Nov 22, 2015
@dsbeach dsbeach deleted the issue2469-Lua-killEvents-not-working branch November 22, 2015 13:26
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

3 participants