Add a Rust example for the NVDA Controller Client #15771

LeonarddeR · 2023-11-11T10:22:29Z

Link to issue number:

None

Summary of the issue:

The rust programming language is becoming more and more relevant, yet an example for the NVDA controller client is missing.

Description of user facing changes

None.

Description of development approach

Add a rust example, based on a workspace with two crates:

nvda-bindgen, containing the logic to create rust bindings based on nvdaController.h
nvda, rustified bindings to the nvda-bindgen crate

Testing strategy:

Test steps are in readme.md

Known issues with pull request:

This is blocked by #15734 for now.

Code Review Checklist:

Documentation:
- Change log entry
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
API is compatible with existing add-ons.
Security precautions taken.

LeonarddeR · 2023-11-11T10:26:01Z

@DataTriny Given your previous investment in NVDA Controller examples and you're much more experienced with Rust than I am, could you have a quick look at the code please?

DataTriny · 2023-11-11T18:43:44Z

Sure I will. I'm away from home until next wednesday though. Le sam. 11 nov. 2023 à 11:26, Leonard de Ruijter ***@***.***> a écrit :

…

@DataTriny <https://github.com/DataTriny> Given your previous investment in NVDA Controller examples and you're much more experienced with Rust than I am, could you have a quick look at the code please? — Reply to this email directly, view it on GitHub <#15771 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC3JTEFWVREFMBIVZ5WLJW3YD5HEJAVCNFSM6AAAAAA7HFBEUCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBWG43TMNRVGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

DataTriny

Minor suggestions from me. Looks good overall!

extras/controllerClient/examples/example_rust/cargo.toml

DataTriny · 2023-11-27T21:42:32Z

extras/controllerClient/examples/example_rust/examples/example_rust.rs

+    0
+}
+
+fn main() {


I'd try to have the main function return Result<()> and remove the calls to Result::unwrap in favor of the ? operator. The call to Result::expect can stay since it provides a clearer error message.

Thanks, good idea!

extras/controllerClient/examples/example_rust/readme.md

DataTriny · 2023-11-27T21:47:01Z

extras/controllerClient/examples/example_rust/src/lib.rs

+pub type OnSsmlMarkReached = onSsmlMarkReachedFuncType;
+
+pub fn test_if_running() -> Result<()> {
+    let res = WIN32_ERROR(unsafe { nvdaController_testIfRunning() });


I think you might be able to come up with a generic function to encapsulate your error handling logic and not repeat it everywhere right?

Sure, will do.

DataTriny · 2023-11-27T21:47:58Z

extras/controllerClient/examples/example_rust/src/lib.rs

+    Ok(())
+}
+
+pub fn braille_message(mesage: &str) -> Result<()> {


Suggested change

pub fn braille_message(mesage: &str) -> Result<()> {

pub fn braille_message(message: &str) -> Result<()> {

DataTriny · 2023-11-27T21:48:43Z

extras/controllerClient/examples/example_rust/src/lib.rs

+}
+
+pub fn braille_message(mesage: &str) -> Result<()> {
+    let message = HSTRING::from(mesage);


Suggested change

let message = HSTRING::from(mesage);

let message = HSTRING::from(message);

Co-authored-by: Arnold Loubriat <[email protected]>

LeonarddeR · 2023-11-28T15:55:15Z

@DataTriny Thanks a lot for your review. I think I have addressed everything.

extras/controllerClient/examples/example_rust/readme.md

DataTriny · 2023-11-28T19:00:57Z

extras/controllerClient/examples/example_rust/src/lib.rs

+
+pub type OnSsmlMarkReached = onSsmlMarkReachedFuncType;
+
+fn is_succes(error: u32) -> Result<()> {


Sadly you have a typo here. Maybe to_result(status: u32) would be more appropriate?

Thanks, I'll rename accordingly.

DataTriny · 2023-11-28T19:02:49Z

extras/controllerClient/examples/example_rust/src/lib.rs

+pub type OnSsmlMarkReached = onSsmlMarkReachedFuncType;
+
+fn is_succes(error: u32) -> Result<()> {
+    let res = WIN32_ERROR(error);


Having read windows-rs's doc, I think you should use WIN32_ERROR::ok instead of your if statement.

That's a very good catch, thanks! Function can now be a one liner.

Co-authored-by: Arnold Loubriat <[email protected]>

DataTriny

I think this is a good start for anyone interested in communicating with NVDA from a Rust program. Good job @LeonarddeR!

LeonarddeR · 2023-11-28T20:56:39Z

Thank you very much @DataTriny!

seanbudd

Note, I don't plan on reviewing the code itself, the only issue I picked up is with the code file licence headers

seanbudd · 2023-11-28T23:51:57Z

extras/controllerClient/examples/example_rust/build.rs

+// Copyright(C) 2023 NV Access Limited, Leonard de Ruijter
+// This file is covered by the GNU Lesser General Public License, version 2.1.
+// See the file license.txt for more details.


could you use the more standard python headers? we don't have a license.txt, we use "COPYING"

There is a license.txt in the controllerClient folder.

Alternatively, if the mention of licence.txt is confusing, you can just use the copyright header for LGPL licensed code from our copyright headers wiki page

Thanks @lukaszgo1, I wasn't aware of these guidelines on the wiki.

seanbudd · 2023-11-30T01:05:14Z

user_docs/en/changes.t2t

@@ -125,6 +125,7 @@ Setting ``numCells`` is still supported for single line braille displays and ``n
 - Device info yielded by ``hwPortUtils.listUsbDevices`` now contain the bus reported description of the USB device (key ``busReportedDeviceDescription``). (#15764, @LeonarddeR)
 - For USB serial devices, ``bdDetect.getConnectedUsbDevicesForDriver`` and ``bdDetect.getDriversForConnectedUsbDevices`` now yield device matches containing a ``deviceInfo`` dictionary enriched with data about the USB device, such as ``busReportedDeviceDescription``. (#15764, @LeonarddeR)
 - When the configuration file ``nvda.ini`` is corrupted, a backup copy is saved before it is reinitialized. (#15779, @CyrilleB79)
+- Added an example to demonstrate using nvdaControllerClient.dll from Rust. (#15771, @LeonarddeR)


Could this be moved to "Changes to the NVDA Controller Client library:"

Add a Rust example for the NVDA Controller Client

740c93c

LeonarddeR added Controller blocked labels Nov 11, 2023

LeonarddeR and others added 2 commits November 24, 2023 08:14

Merge remote-tracking branch 'origin/master' into rust

30475aa

Use .lib and .h without explicit copying

316d989

LeonarddeR removed the blocked label Nov 27, 2023

LeonarddeR added 5 commits November 27, 2023 18:26

use just one crate

7546e6f

Don't use pub(crate)

c92ca91

Update documentation

ecf8814

Add copyright headers

fbf2f77

Update cargo.toml

4299827

LeonarddeR marked this pull request as ready for review November 27, 2023 19:01

LeonarddeR requested a review from a team as a code owner November 27, 2023 19:01

LeonarddeR requested a review from seanbudd November 27, 2023 19:01

Cleanup .gitignore

910a0e6

DataTriny suggested changes Nov 27, 2023

View reviewed changes

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Nov 27, 2023

seanbudd marked this pull request as draft November 28, 2023 00:11

LeonarddeR and others added 4 commits November 28, 2023 12:38

First batch of review actions

beafb9a

Update extras/controllerClient/examples/example_rust/readme.md

0dd0f18

Co-authored-by: Arnold Loubriat <[email protected]>

Return Result and swap unwrap for question mark operator

8b60fcd

Last review action

df2937a

LeonarddeR marked this pull request as ready for review November 28, 2023 17:41

DataTriny suggested changes Nov 28, 2023

View reviewed changes

LeonarddeR and others added 2 commits November 28, 2023 21:11

Update extras/controllerClient/examples/example_rust/readme.md

27443d2

Co-authored-by: Arnold Loubriat <[email protected]>

Review actions

fb63082

DataTriny approved these changes Nov 28, 2023

View reviewed changes

seanbudd reviewed Nov 28, 2023

View reviewed changes

LeonarddeR added 3 commits November 29, 2023 13:20

Update changelog

c417b0d

Update copyright headers according to wiki

bd23536

Merge remote-tracking branch 'origin/master' into rust

3504238

seanbudd reviewed Nov 30, 2023

View reviewed changes

LeonarddeR added 2 commits November 30, 2023 06:59

Merge remote-tracking branch 'origin/master' into rust

3ec4615

Move changelog entry

ecade7c

seanbudd merged commit 021c13d into nvaccess:master Nov 30, 2023

nvaccessAuto added this to the 2024.1 milestone Nov 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a Rust example for the NVDA Controller Client #15771

Add a Rust example for the NVDA Controller Client #15771

LeonarddeR commented Nov 11, 2023 •

edited

Loading

LeonarddeR commented Nov 11, 2023

DataTriny commented Nov 11, 2023 via email

DataTriny left a comment

DataTriny Nov 27, 2023

LeonarddeR Nov 28, 2023

DataTriny Nov 27, 2023

LeonarddeR Nov 28, 2023

DataTriny Nov 27, 2023

DataTriny Nov 27, 2023

LeonarddeR commented Nov 28, 2023

DataTriny Nov 28, 2023

LeonarddeR Nov 28, 2023

DataTriny Nov 28, 2023

LeonarddeR Nov 28, 2023

DataTriny left a comment

LeonarddeR commented Nov 28, 2023

seanbudd left a comment •

edited

Loading

seanbudd Nov 28, 2023

LeonarddeR Nov 29, 2023

lukaszgo1 Nov 29, 2023

LeonarddeR Nov 29, 2023

seanbudd Nov 30, 2023

	pub fn braille_message(mesage: &str) -> Result<()> {
	pub fn braille_message(message: &str) -> Result<()> {

	let message = HSTRING::from(mesage);
	let message = HSTRING::from(message);


		pub type OnSsmlMarkReached = onSsmlMarkReachedFuncType;

		fn is_succes(error: u32) -> Result<()> {

Add a Rust example for the NVDA Controller Client #15771

Add a Rust example for the NVDA Controller Client #15771

Conversation

LeonarddeR commented Nov 11, 2023 • edited Loading

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Code Review Checklist:

LeonarddeR commented Nov 11, 2023

DataTriny commented Nov 11, 2023 via email

DataTriny 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

LeonarddeR commented Nov 28, 2023

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

DataTriny left a comment

Choose a reason for hiding this comment

LeonarddeR commented Nov 28, 2023

seanbudd left a comment • edited Loading

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 11, 2023 •

edited

Loading

seanbudd left a comment •

edited

Loading