Will require reference counting on modifiers at the device level. Preferably the SystemInput base class should handle it so every sys input device doesn't need to implement.

Created attachment 86248 [details] [review]
patch for Modifier keys cannot be removed

Comment on attachment 86248 [details] [review]
patch for Modifier keys cannot be removed

>   def removeModifier(self, code):
>     '''
>-    Removes an action code from the modifiers dictionary.
>+    Removes an action code from the modifiers dictionary if it is no longer
>+    in use.  Otherwise, it decrements the reference count.
>     
>     @param code: Action code to unregister as a modifier
>     @type code: integer
>-    @raise KeyError: When the action code to remove is not registered as a 
>-      modifier
>     '''
>-    del self.modifiers[code]
>+    try:
>+      self.modifiers[code] -= 1
>+      if self.modifiers[code] <= 0:
>+        del self.modifiers[code]
>+    except:
>+      pass
>     

except should catch specific exception(s) like KeyError. Any others?

>   def clearModifiers(self):
>     '''

Need to do anything new in clear?

>@@ -149,11 +150,18 @@
>     for tasks in (self.named_tasks.values(), self.keyed_tasks.values()):
>       map(Task.Task.close, tasks)
>       map(Task.Task.postClose, tasks)
>+      
>+    # unregister modifiers
>+    for dev, codes in self.registered_modifiers.iteritems():
>+      for code in codes:
>+        self.removeInputModifiers(dev, code)
>+      
>     # throw away all references
>     self.named_tasks = {}
>     self.keyed_tasks = {}
>     self.event_tasks = {}
>     self.commands = {}
>+    self.registered_modifiers = {}
>     
>   def setIdealOutput(self, *capabilities):
>     '''
>Index: src/Task/Tools/Input.py
>===================================================================
>--- src/Task/Tools/Input.py	(revision 709)
>+++ src/Task/Tools/Input.py	(working copy)
>@@ -64,7 +64,29 @@
>     except AttributeError:
>       return
>     map(func, codes)
>-
>+    # add modifiers to device specific list for later removal
>+    try:
>+      self.registered_modifiers[dev].extend(codes)
>+    except KeyError:
>+      self.registered_modifiers[dev] = list(codes)

Can't do self.registered_modifiers. What if it's a Task calling this method? That dict is only in the perk. Code should probably be self.perk.registered_modifiers etc.

But I'm not sure you want to be doing this anyways. How does this affect the reference count maintained by the device? This only changes the local Perk dictionary. Doesn't the device need to be informed too?

>+    try:
>+      func = dev.removeModifier 
>+    except AttributeError:
>+      return
>+    map(func, codes)
>+    

Opposite problem here. Modifiers are getting removed from the device, but never from the Perk dictionary.

Responses to comment #3

> except should catch specific exception(s) like KeyError. Any others?
done

> Need to do anything new in clear?
No, that should work.  Only the key was being used in the self.modifiers dictionary.  I used the value part to hold an integer reference count.

> Can't do self.registered_modifiers. What if it's a Task calling this method?
> That dict is only in the perk. Code should probably be
> self.perk.registered_modifiers etc.
sure enough.  good catch.

> But I'm not sure you want to be doing this anyways. How does this affect the
> reference count maintained by the device? This only changes the local Perk
> dictionary. Doesn't the device need to be informed too?
> Opposite problem here. Modifiers are getting removed from the device, but never
> from the Perk dictionary.
both should be good in new patch

Monitoring self.modifiers in SystemInput.removeModifier/addModifier shows that modifiers are being reference counted and removed properly.

Created attachment 86649 [details] [review]
patch for Modifier keys cannot be removed

Comment on attachment 86649 [details] [review]
patch for Modifier keys cannot be removed

>+    self.registered_modifiers = {}

This dictionary needs to store weak references to devices. Otherwise, the user will be unable to switch output devices at runtime.

>+    self.registered_modifiers = {}

Again, make it a weak dict when you clean up.

>+    # add modifiers to device specific list for later removal
>+    self.perk.registered_modifiers.setdefault(dev, []).extend(codes)

Do not access Perk instance variables directly, in case our implementation of modifier reference counting should ever change. Create a method on the Perk which this method can call to add modifiers.

>+      for c in codes:
>+        self.perk.registered_modifiers[dev].remove(c)

Same here. Perk method to remove modifiers is needed.

Created attachment 87087 [details] [review]
patch for Modifier keys cannot be removed

Recommended changes contained in new patch.

Committed latest patch with fixes to documentation and better error handling.

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.