Discussion:
[BitPim-devel] Error getting lost
Roger Binns
2007-01-22 06:40:48 UTC
Permalink
An exception is being swallowed that shouldn't. I couldn't work out the
cause due to all the layers of indirection and threads that go on for at
commands. To repeat do this on Linux:

- - Connect phone and have BitPim talk to it
- - Unplug phone
- - Try to do a phone operation

You get back an error saying that the phone didn't respond. That is not
remotely true - the write on the device will be giving back some error -
ie the phone didn't get an opportunity to respond. We really should be
raising the error, not swallowing and giving back something generic.

On the threading side, creating a new thread per AT command seems nuts.
There is an easier way. Use a Queue.Queue and put the functions and
args to execute into the queue, and then have the thread put the result
back on a different (paired) queue. The result can be a tuple of the
result and the exception (ie one of them is None). The exception should
include the exception value, type and traceback and can be re-raised in
the calling thread. The workerthread stuff in the gui already does this.

If there is a danger of a thread getting stuck then we may be able to
use native code. Worst case, after putting a workitem on the queue,
wait a while and look at the queue length. If it isn't empty then
create a new thread and ignore the jammed one.

The pseudo-code can look something like this:

workq.put( func, args, kwargs)
again:
result, exception = resultq.get(10 seconds)
if timedout:
if !workq.empty:
create new worker thread
goto again
handle timeout (make a new workq, resultq etc)
if exception:
raise exception[0], exception[1], exception[2]
return result

Roger
Joe Pham
2007-01-22 22:08:03 UTC
Permalink
I agree that this can be improved. How do you want to work this?
Do/can you also take on this?

-Joe Pham




_____________________________________________________________________
Interested in getting caught up on today's news?
Click here to checkout USA TODAY Headlines.
http://track.netzero.net/s/lc?s=198954&u=http://www.usatoday.com/news/front.htm?csp=24
Loading...