Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when writing text with special character #132

Closed
OlivierJourdan opened this issue Jan 12, 2023 · 8 comments
Closed

Error when writing text with special character #132

OlivierJourdan opened this issue Jan 12, 2023 · 8 comments

Comments

@OlivierJourdan
Copy link

G-CLI version 3.0.0 beta1


I have the following error when writing a text with a special character with the Write String function:

Comms Error: Message contents cannot be read as UTF8

It would be nice to find a fix when the string you want to write is not a constant (file path, for example).

NOTE: I already implemented a quick and dirty workaround—no emergency to implement a fix for me.

For the record, here is my fix
image

@JamesMc86
Copy link
Collaborator

JamesMc86 commented Jan 19, 2023

Thanks Olivier - one of those simple things that are so complicated!

I'll take a look at what I can do to support the extended ASCII tables.

You probably have more experience with this than myself so any hints? I presume LabVIEW just uses the active code page in Windows?

@OlivierJourdan
Copy link
Author

I didn't dive into the code to see where the issue was. So, I haven't ideas how to fix it properly.
From a user standpoint, I'm fine if extended characters are not displayed. I'm sure your time could be more valuable on other issues.
IMO, the important thing is not to be stopped by an extended character.

@JamesMc86
Copy link
Collaborator

Ah no worries, wasn't sure if it is something you might have come across elsewhere. Turns out extended ASCII like LabVIEW is a minefield for compatibility

We can definitely make it robust to this as you say. There are 3 options to consider:

  1. Simplest is to do a lossy conversion to UTF8 in Rust.
  2. We can convert to UTF8 in LabVIEW. There appears to be a function for this in described at https://knowledge.ni.com/KnowledgeArticleDetails?id=kA00Z00000159jOSAQ&l=en-GB
  3. We can convert from ExtendedASCII to UTF8 in rust. There is an encoding library but we would have to determine the current code page from the Windows API - and consider cross platform concerns

I'm inclined towards 2 although sounds like the tools provided just wrap the Windows API so perhaps should be 3 and we can use them to ensure we are calling the correct methods.

@OlivierJourdan
Copy link
Author

I'm not sure 2 will work. If I remember, that was the first thing I tried, and it was unsuccessful.

@JamesMc86
Copy link
Collaborator

I'll have a play - I've just looked in the NI Unicode tools and there is a wrapper for a private primitive which is text to UTF-8 and appears to be cross-platform.

Sounds like what we would need but perhaps there are limitations?

Alternatively I can copy their method for ASCII to UTF-16 using Windows calls at least - I can see the correct options now and wrap then on the Rust side where there are easier conversions from UTF-16 to UTF-8 (the internal representation required)

@OlivierJourdan
Copy link
Author

Nice. If you want me to test something, just let me know.

JamesMc86 added a commit that referenced this issue Jan 20, 2023
Added UTF8 conversions at LabVIEW boundaries so all data on the wire is
UTF8 for the proxy.

Additional test file shows the impact of non-ascii in filename which
still requires a fix and so is commented out of the integration tests.

for #132
@JamesMc86
Copy link
Collaborator

The next release should fix these issues. Hopefully, it will appear on Github today!

JamesMc86 added a commit that referenced this issue Feb 16, 2023
Just to handle another case raised in #132

non-ascii is valid in filename but affects matches. This replaces them
in the service name by underscores.

NOTE: This does risk a name collision but the risks of this seem very
low.
@OlivierJourdan
Copy link
Author

Excellent! Thanks for this @JamesMc86 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants