Add a Rust example for the NVDA Controller Client by LeonarddeR · Pull Request #15771 · nvaccess/nvda

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!

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

+    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!

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

+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

+    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

+}
+
+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 <datatriny@gmail.com>

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

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

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

+
+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

+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 <datatriny@gmail.com>

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

+// 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

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 <datatriny@gmail.com>

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 <datatriny@gmail.com>

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

Comment thread user_docs/en/changes.t2t Outdated

seanbudd Nov 30, 2023

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

LeonarddeR deleted the rust branch August 23, 2025 06:28

	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<()> {

Uh oh!

Conversation

LeonarddeR commented Nov 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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:

Uh oh!

LeonarddeR commented Nov 11, 2023

Uh oh!

DataTriny commented Nov 11, 2023 via email

Uh oh!

DataTriny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeonarddeR commented Nov 28, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DataTriny left a comment

Choose a reason for hiding this comment

Uh oh!

LeonarddeR commented Nov 28, 2023

Uh oh!

seanbudd left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

LeonarddeR commented Nov 11, 2023 •

edited

Loading

seanbudd left a comment •

edited

Loading