Project

General

Profile

Actions

Bug #1102

closed

TouchProcessor fails on pinet to exit or notify on exit when running on pistore.

Added by Hammel 12 months ago. Updated 8 days ago.

Status:
Closed
Priority:
Immediate
Assignee:
Target version:
Start date:
04 Mar 2024
Due date:
% Done:

100%

Estimated time:
Severity:
01 - Critical

Description

Failure on pinet shutdown on a pistore:

piboxTouchShutdownProcessor[touchProcessor.c:619] ERROR Timed out waiting on touchProcessor thread to shut down.

Doesn't happen all the time and the timeout eventually clears the problem.

Actions #1

Updated by Hammel 10 months ago

  • Severity changed from 03 - Medium to 01 - Critical
Actions #2

Updated by Hammel 8 months ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 30

This might be due to the animation timer interfering with the SIGINT processing. There was no call to g_source_remove() in pinet.c on shutdown.

This is fixed in my sandbox but needs to be tested thoroughly on hardware.

Note that the following all need updates related to the SIGINT call.
  • picam
  • musicfe
  • pipics

This means they all need this code on shutdown.

    if ( isCLIFlagSet(CLI_TOUCH) )
    {   
        /* SIGINT is required to exit the touch processor thread. */
        raise(SIGINT);
        piboxTouchShutdownProcessor();
    }

And the following need updates to disable a timer before the SIGINT call.
  • pistore
  • pinet
Actions #3

Updated by Hammel 8 months ago

  • Subject changed from TouchProcessor fails to exit or notify on exit on pistore. to TouchProcessor fails on pinet to exit or notify on exit when running on pistore.
Actions #4

Updated by Hammel 8 months ago

  • % Done changed from 30 to 50

All changes are implemented but all changes need to be tested on hardware.

Kiosk can be used to test the following:
  • pipics: passed
PiSentry can be used to test the following:
  • picam - passed, after fixing location of touch handler shutdown.
  • pinet - fail
Media System can test the following, but there is no touchscreen use case for them so there shouldn't be an issue.
  • Musicfe: Passed
PiStore system can be used to test the following:
  • pistore: passed
Actions #5

Updated by Hammel 9 days ago

  • % Done changed from 50 to 80

pinet is still failing with this even though the code looks correct. It still times out waiting on the touch processor.

I'm not sure I really like this mechanism as it relies on a signal in stead of a more robust mechanism like a thread variable or semaphore.

In any case, all of the code can be committed and pushed. Then I just need to fix pinet from here.

Actions #6

Updated by Hammel 9 days ago

Possible issue: picam doesn't have ANY signal handling. Just add the same thing I put in pinet and see if that helps.

Update: looks like ONLY pinet has signal handling. pistore doesn't have it. musicfe doesn't have it. pipics doesn't have it. In fact mfe and pipics have it but it's been disabled (#if 0'd). Still, I want to try adding it to see what happens.

The problem here is that ts_read() in piboxlib:touchProcessor() is blocking and will only interrupt on SIGINT. We can make it non-blocking with a call to ts_setup but I don't know if the code is setup to handle non-blocking reads. I'll have to experiment. If we can use non-blocking calls then we can use a semaphore to shutdown the touchprocessor, which will be much cleaner than SIGINT.

Actions #7

Updated by Hammel 8 days ago

touchProcessor() calls ts_setup(xxx,0) which means blocking reads. If it would call it with ts_setup(xxx,1) then it would be non-blocking. touchProcessor() calls ts_read(xxx,1) meaning it just wants one entry so the code right after that, if (ret==0), handles a non-blocking return.

So technically, all that should be required is to have touchProcessor() call ts_setup(xxx,1) and it should just work. I MIGHT need to put a minor delay in the if(ret==0) block to keep touchProcessor() from hogging CPU time, however.

If I do this then the shutdownProcessor() call can just use a semaphore to break out of the touchProcessor() loop.

To test: make the mods to libpibox and push the .opk to the node to test. It either works or it doesn't.

Actions #8

Updated by Hammel 8 days ago

  • Status changed from In Progress to Closed
  • % Done changed from 80 to 100

Wow. Fixed this!

The fix wasn't to use a semaphore. There was already loop management integrated into touchProcessor(), I just wasn't using it correctly. When fixed and switched to non-blocking reads, the whole thing just works a ton better.

Tested with picam, pinet, and pistore. All of which now exit much faster and cleanly.

Code (piboxlib:touchProcessor.c was the only change) committed and pushed.

Closing issue.

Actions

Also available in: Atom PDF