BrailleNote: support for QT and Apex BT scroll wheel #6316

josephsl · 2016-08-30T08:23:58Z

This pull request addresses the following issues:

BrailleNote: Support scroll wheel on Apex BT #5992: Support for Apex BT's scroll wheel.
Add support for BrailleNote QT basic mode #5993: Support for BraileNote QT (basic mode) when used as a braille display with NVDA.

All changes are documented in the user guide.

Suggested what's new:
Category: New features
Added support for BrailleNote QT and Apex BT's scroll wheel when BrailleNote is used as a braille display with NVDA.

Thanks.

…ss#5992. Somehow, forgot to check that Rui used a tuple to specify key assignments. Now utilized properly to assign multiple gestures to a command. Also took this time to add codes for various QT keys so NVDA can properly recognize them.

jcsteh · 2016-09-21T03:36:49Z

Thanks for your work. I have a few overall questions/concerns about this first before I do a formal review.

Keyboard Emulation

Is your intention to support keyboard emulation, but you weren't able to manage this yet? Or is there any reason you don't want to support this? Supporting that will require a change to the approach you're using. I think it might be worth considering that now, rather than having to do a major refactor later. Following are some thoughts on that.

Rather than emitting BrailleNote gestures and mapping them to key presses, I think it'd be simpler to just emit most keys as KeyboardInputGestures. The QT keys are actually Windows vk codes. So, for most key presses, you would do the following:

Map the QT modifiers to Windows vk codes; e.g. control to VK_CONTROL.
Instantiate a KeyboardInputGesture. Some notes on the arguments:
- modifiers: The vk codes you mapped in step 1.
- vkCode: The key code you got from the BrailleNote.
- scanCode: None.
- isExtended: Normally True, unless the vk code is 0xD (enter), in which case False.
Call inputCore.manager.emulateGesture with this gesture.

For most keys involving the QT function or read modifiers, you'd still want to produce BrailleNote specific gestures as you do now, as these are specific to the BrailleNote. To convert the vk code to a name, you can use KeyboardInputGesture.getVkName.

You might want to map control+read to the Windows key and then just treat it as a KeyboardInputGesture as above.

Handling Key Packets

At present, you read the extra bytes for QT_MOD_TAG in _readKeys. I suggest moving this to _readPacket, as this is where "reading" is done. Also, data is really just a second argument, so perhaps call it arg2.

josephsl · 2016-09-21T04:59:20Z

Hi,

When to read packet: I'll do some experiments this weekend to see what's possible and moving the parsing routine to readPacket function.
QT emulation: what's making this confusing is the fact that Humanware has implemented PC keyboard emulation mode. Effectively, a QT user can switch between standard terminal mode (the one currently implemented) and PC keyboard mode where some combinations map directly to key presses.
QT terminal commands: My intention is to provide similar experience to that of BT users, but considering that people would like to type on QT keyboard and to serve as an example for other QWERTY keyboard notetakers, I'd be happy to let the branch support suggestions as outlined above (scan code/VK code entry).

Thanks.

jcsteh · 2016-09-21T05:14:05Z

When to read packet: I'll do some experiments this weekend to see what's possible and moving the parsing routine to readPacket function.

I'd have readPacket return three values, with the third being None for most commands except the QT modifier command.

QT emulation: what's making this confusing is the fact that Humanware has implemented PC keyboard emulation mode. Effectively, a QT user can switch between standard terminal mode (the one currently implemented) and PC keyboard mode where some combinations map directly to key presses.

Yes, control mode and input mode according to the protocol documentation. I don't quite understand the point of input mode, though, because control mode also provides key presses. if anything, control mode is actually more "raw"; it gives you real modifiers. From what I've read, Window-Eyes allows QWERTY input in both, whereas JAWS only allows QWERTY input in input mode.

QT terminal commands: My intention is to provide similar experience to that of BT users, but considering that people would like to type on QT keyboard and to serve as an example for other QWERTY keyboard notetakers, I'd be happy to let the branch support suggestions as outlined above (scan code/VK code entry).

I wouldn't worry about serving as an example. If you don't feel this is appropriate, don't do it; it was just a suggestion because I thought people might like to be able to type. Different QWERTY notetakers do things differently anyway. For example, Papenmeier actually passes entirely raw key presses, right down to key ups and key downs for modifiers, whereas BrailleNote does special handling for modifiers.

…nvaccess#5992 Comment from Jamie Teh (Nv Access): there's no need to call read packet again if we're dealing with QT, so handle this from read packet directly. Thus readPacket returns three things: command, arg (dot mask on BT/command for QT), arg2 (char from QT).

Instead of relying on a dedicated QT key parser, handle parsing directly in readPacket. Also simplifies gesture ID assignment, as the packet will tell the gesture constructor as to what key it is.

jcsteh

Thanks!

Did you decide against doing QWERTY emulation and mapping vk codes using the existing KeyboardInputGesture stuff as I suggested? If so, that's fine, but be aware that emulation will require a significant refactor later.

jcsteh · 2016-09-26T23:22:25Z

source/brailleDisplayDrivers/brailleNote.py

+		if command == QT_MOD_TAG:
+			key = self._serial.read(2)[-1]
+			arg2 = _qtKeys[ord(key)] if ord(key) in _qtKeys else key
+		else:


You could simplify this to: arg2 = _qtKeys.get(ord(key), key)

jcsteh · 2016-09-26T23:23:48Z

source/brailleDisplayDrivers/brailleNote.py


 	# Data is invoked if we're dealing with a BrailleNote QT.
-	def _dispatch(self, command, arg, data=None):
+	def _dispatch(self, command, arg, arg2=None):


data -> arg2. Also, maybe passed instead of invoked?

jcsteh · 2016-09-27T00:02:53Z

source/brailleDisplayDrivers/brailleNote.py

+		if self.qt:
+			self.id = qtData if qtMod == 0 else "+".join(("+".join(names), qtData))
+		else:


Gesture identifiers actually must be in Python set order; you can't reorder things. Otherwise, they might not match gesture maps. If you want to reorder them for display reasons, you will need to override displayName and logIdentifier. If you want to do that, you'll need a helper method which all of them call which returns the names in the correct order (list not set).

jcsteh · 2016-09-27T00:06:46Z

user_docs/en/userGuide.t2t

@@ -2093,6 +2100,39 @@ Please check your BrailleNote's documentation to find where these keys are locat
 | Windows key | space+dot2+dot4+dot5+dot6 (space+w) |
 | Alt key | space+dot1+dot3+dot4 (space+m) |
 | Toggle input help | space+dot2+dot3+dot6 (space+lower h) |
+
+Following are commands assigned to BrailleNote QT.
+%kc:beginInclude


A kc include block was already started earlier.

You probably want to include the sentence explaining the purpose of each table in the kc document anyway, so just leave the kc include block open from before.

Should the braille dot commands be split out of the main commands into a separate table, since I assume you can't execute those on a QT?

You can ask QT to emulate braille dots by pressing a key (it's a toggle). I can clarify this.

jcsteh · 2016-09-27T00:07:58Z

user_docs/en/userGuide.t2t

+| Toggle input help | read+1 |
+
+Following are commands assigned to the scroll wheel:
+%kc:beginInclude


See above re kc include block.

jcsteh · 2016-09-27T00:08:51Z

user_docs/en/userGuide.t2t

+%kc:beginInclude
+|| Name | Key |
+| NvDA menu | read+n |
+| Up arrow key | up arrow |


The convention in NVDA is normally to write keys in camel case; e.g. downArrow instead of down arrow.

…onary key lookup. re nvaccess#5993 A tip from Jamie Teh (NV Access0: use dict.get to simplify the turnary operation. Also, BNQT ticket is 5993, not 5992, and correct comment text.

Review from Jamie Teh (NV Access): * QT can emulate a BrailleNote BT, hence clarify this. * Removed redundant kcinclude statement.

… gesture identifier right. nvaccess#5993 Comment from Jamie Teh (NV Access): use sets instead of turning that into a list just for formatting purposes (and turns out this is unnecessary).

LeonarddeR · 2017-11-08T20:20:56Z

This has now merge conflicts since this driver is hwIo based.

josephsl · 2017-11-08T20:29:34Z

Hi, looks like I need to do a complete rebase. Thanks for telling me about this – will get to it next week (busy this weekend). From: Leonard de Ruijter [mailto:notifications@github.com] Sent: Wednesday, November 8, 2017 12:21 PM To: nvaccess/nvda <nvda@noreply.github.com> Cc: Joseph Lee <joseph.lee22590@gmail.com>; Author <author@noreply.github.com> Subject: Re: [nvaccess/nvda] BrailleNote: support for QT and Apex BT scroll wheel (#6316) This has now merge conflicts since this driver is hwIo based. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6316 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkHHq3ZUlKbE30XkD0hRJrsseNtSwks5s0g0qgaJpZM4JwRDh> .

# Conflicts: # source/appModules/win32calc.py # source/brailleDisplayDrivers/brailleNote.py

feerrenrut · 2018-04-11T06:29:51Z

@LeonarddeR Would you mind reviewing this when you have a chance?

LeonarddeR · 2018-04-11T10:01:34Z

@josephsl: Could you please resolve another merge conflict?

LeonarddeR

Looks very good overall. Apart from the inline comments, could you please also revert the mode switch that occurred for appModules/win32calc.py

LeonarddeR · 2018-04-11T10:13:49Z

source/brailleDisplayDrivers/brailleNote.py

@@ -74,13 +84,48 @@
 	0 : "space"
 }

+# Scroll wheel components (Apex BT)
+_scrWheel = ("wcounterclockwise", "wclockwise", "wup", "wdown", "wleft", "wright", "wcenter")


To improve consistency, could you rename these to wCounterclockwise, wClockwise, wUp, etc?

LeonarddeR · 2018-04-11T10:14:29Z

source/brailleDisplayDrivers/brailleNote.py

+	186:"semi",
+	187:"equals",
+	188:"comma",
+	189:"hyphen",


I'd call this dash

LeonarddeR · 2018-04-11T10:15:20Z

source/brailleDisplayDrivers/brailleNote.py

+	187:"equals",
+	188:"comma",
+	189:"hyphen",
+	190:"period",


I'd call this dot.

LeonarddeR · 2018-04-11T10:15:44Z

source/brailleDisplayDrivers/brailleNote.py

+	189:"hyphen",
+	190:"period",
+	191:"slash",
+	192:"graav",


I'd call this grave

LeonarddeR · 2018-04-11T10:31:53Z

source/brailleDisplayDrivers/brailleNote.py

 				# Force dot8 here, although it should be already there
 				arg |= DOT_8
 			gesture = InputGesture(dots=arg, space=space)
+		elif command == QT_MOD_TAG:
+			# BrailleNote QT
+			gesture = InputGesture(qtMod=arg, qtData=arg2)


Have you actually considered making this a keyboardHandler.KeyboardInputGesture?

josephsl · 2018-04-11T12:59:47Z

Hi, I will do so this weekend. Thanks. From: Leonard de Ruijter <notifications@github.com> Sent: Wednesday, April 11, 2018 3:02 AM To: nvaccess/nvda <nvda@noreply.github.com> Cc: Joseph Lee <joseph.lee22590@gmail.com>; Mention <mention@noreply.github.com> Subject: Re: [nvaccess/nvda] BrailleNote: support for QT and Apex BT scroll wheel (#6316) @josephsl <https://github.com/josephsl> : Could you please resolve another merge conflict? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6316 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkJ4RSD6fNNJf2cqoZx0rW62IpL55ks5tndSCgaJpZM4JwRDh> .

josephsl · 2018-04-11T13:02:11Z

Hi, not exactly, as I haven’t delved far into this. Thanks. From: Leonard de Ruijter <notifications@github.com> Sent: Wednesday, April 11, 2018 3:33 AM To: nvaccess/nvda <nvda@noreply.github.com> Cc: Joseph Lee <joseph.lee22590@gmail.com>; Mention <mention@noreply.github.com> Subject: Re: [nvaccess/nvda] BrailleNote: support for QT and Apex BT scroll wheel (#6316) @LeonarddeR commented on this pull request.

_____ In source/brailleDisplayDrivers/brailleNote.py <#6316 (comment)> :

# Force dot8 here, although it should be already there

arg |= DOT_8 gesture = InputGesture(dots=arg, space=space) + elif command == QT_MOD_TAG: + # BrailleNote QT + gesture = InputGesture(qtMod=arg, qtData=arg2) Have you actually considered making this a keyboardHandler.KeyboardInputGesture? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6316 (review)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkI9NNmdmJCo7qmzve3DDwcPQO6mQks5tndvHgaJpZM4JwRDh> .

# Conflicts: # user_docs/en/userGuide.t2t

…5992. Commented by Leonard de Ruijter: rename QT key names. Update copyright years.

LeonarddeR · 2018-04-13T11:32:48Z

@josephsl: Were you also intending to fix de mode change for win32calc.py?

josephsl · 2018-04-13T16:41:26Z

Hi, ah yes, about to do that – chmod. Thanks. From: Leonard de Ruijter <notifications@github.com> Sent: Friday, April 13, 2018 4:33 AM To: nvaccess/nvda <nvda@noreply.github.com> Cc: Joseph Lee <joseph.lee22590@gmail.com>; Mention <mention@noreply.github.com> Subject: Re: [nvaccess/nvda] BrailleNote: support for QT and Apex BT scroll wheel (#6316) @josephsl <https://github.com/josephsl> : Were you also intending to fix de mode change for win32calc.py? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6316 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkDPgLk9eO_CC0jIKtXcUt4lQAM37ks5toIzjgaJpZM4JwRDh> .

derekriemer · 2018-06-10T20:59:06Z

What's the status of this?

josephsl · 2018-06-10T21:32:40Z

Hi, waiting for 2018.3 development to begin so we can come back to this. Thanks. From: Derek Riemer <notifications@github.com> Sent: Sunday, June 10, 2018 1:59 PM To: nvaccess/nvda <nvda@noreply.github.com> Cc: Joseph Lee <joseph.lee22590@gmail.com>; Mention <mention@noreply.github.com> Subject: Re: [nvaccess/nvda] BrailleNote: support for QT and Apex BT scroll wheel (#6316) What's the status of this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6316 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkKGNWwuNDc2OucAyztVjWKoI0YRoks5t7YidgaJpZM4JwRDh> .

josephsl added 4 commits May 24, 2016 02:36

BrailleNote: Support scroll wheel on Apex BT. re nvaccess#5992

197b8d7

BrailleNote: Support BrailleNote QT basic mode. re nvaccess#5993

348ab82

Merge branch 'master' into i5992

ee884d5

josephsl added 2 commits September 22, 2016 23:43

BrailleNote QT: QT key parser refactor.

600636a

Instead of relying on a dedicated QT key parser, handle parsing directly in readPacket. Also simplifies gesture ID assignment, as the packet will tell the gesture constructor as to what key it is.

jcsteh requested changes Sep 27, 2016

View reviewed changes

josephsl added 3 commits September 26, 2016 18:26

BrailleNote QT: comment text change, correct ticket, simplified dicti…

11793dd

…onary key lookup. re nvaccess#5993 A tip from Jamie Teh (NV Access0: use dict.get to simplify the turnary operation. Also, BNQT ticket is 5993, not 5992, and correct comment text.

User guide: cosmetics. re nvaccess#5992

58f5f9a

Review from Jamie Teh (NV Access): * QT can emulate a BrailleNote BT, hence clarify this. * Removed redundant kcinclude statement.

BrailleNote QT: use set instead of worrying too much about formatting…

2933f19

… gesture identifier right. nvaccess#5993 Comment from Jamie Teh (NV Access): use sets instead of turning that into a list just for formatting purposes (and turns out this is unnecessary).

josephsl added 3 commits November 16, 2017 21:38

Merge master

ae0fa06

Merge branch 'master' into i5992

269590d

# Conflicts: # source/appModules/win32calc.py # source/brailleDisplayDrivers/brailleNote.py

Readjust commands

d9498a2

feerrenrut requested a review from LeonarddeR April 11, 2018 06:28

LeonarddeR suggested changes Apr 11, 2018

View reviewed changes

LeonarddeR reviewed Apr 11, 2018

View reviewed changes

josephsl added 3 commits April 11, 2018 20:46

Merge branch 'master' into i5992

9190e49

# Conflicts: # user_docs/en/userGuide.t2t

BrailleNote: copyright year update, QT key name changes. Re nvaccess#…

5aee821

…5992. Commented by Leonard de Ruijter: rename QT key names. Update copyright years.

BrailleNote: rename scroll wheel constant strings. Re nvaccess#5992.

cec3188

Restore executable permission to win32calc

466b8fb

LeonarddeR approved these changes Apr 30, 2018

View reviewed changes

LeonarddeR pushed a commit that referenced this pull request Apr 30, 2018

Incubates #6316

f46ab16

nvaccessAuto added the incubating label Apr 30, 2018

nvaccessAuto assigned LeonarddeR Apr 30, 2018

michaelDCurran merged commit 30cd6b8 into nvaccess:master Jun 13, 2018

nvaccessAuto removed the incubating label Jun 13, 2018

nvaccessAuto added this to the 2018.3 milestone Jun 13, 2018

ehollig mentioned this pull request Aug 27, 2018

BrailleNote: Support scroll wheel on Apex BT #5992

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BrailleNote: support for QT and Apex BT scroll wheel #6316

BrailleNote: support for QT and Apex BT scroll wheel #6316

josephsl commented Aug 30, 2016

jcsteh commented Sep 21, 2016

josephsl commented Sep 21, 2016

jcsteh commented Sep 21, 2016

jcsteh left a comment

jcsteh Sep 26, 2016

jcsteh Sep 26, 2016

jcsteh Sep 27, 2016

jcsteh Sep 27, 2016

josephsl Sep 27, 2016

jcsteh Sep 27, 2016

jcsteh Sep 27, 2016

LeonarddeR commented Nov 8, 2017

josephsl commented Nov 8, 2017 via email

feerrenrut commented Apr 11, 2018

LeonarddeR commented Apr 11, 2018

LeonarddeR left a comment

LeonarddeR Apr 11, 2018

LeonarddeR Apr 11, 2018

LeonarddeR Apr 11, 2018

LeonarddeR Apr 11, 2018

LeonarddeR Apr 11, 2018

josephsl commented Apr 11, 2018 via email

josephsl commented Apr 11, 2018 via email

LeonarddeR commented Apr 13, 2018

josephsl commented Apr 13, 2018 via email

derekriemer commented Jun 10, 2018

josephsl commented Jun 10, 2018 via email

BrailleNote: support for QT and Apex BT scroll wheel #6316

BrailleNote: support for QT and Apex BT scroll wheel #6316

Conversation

josephsl commented Aug 30, 2016

jcsteh commented Sep 21, 2016

Keyboard Emulation

Handling Key Packets

josephsl commented Sep 21, 2016

jcsteh commented Sep 21, 2016

jcsteh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeonarddeR commented Nov 8, 2017

josephsl commented Nov 8, 2017 via email

feerrenrut commented Apr 11, 2018

LeonarddeR commented Apr 11, 2018

LeonarddeR left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephsl commented Apr 11, 2018 via email

josephsl commented Apr 11, 2018 via email

LeonarddeR commented Apr 13, 2018

josephsl commented Apr 13, 2018 via email

derekriemer commented Jun 10, 2018

josephsl commented Jun 10, 2018 via email