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

fix(string-decode): add decoding support for ISO 8859-1 #35

Merged
merged 2 commits into from
Jul 7, 2017

Conversation

kfenner
Copy link
Collaborator

@kfenner kfenner commented Jul 6, 2017

ISO 8859-1 is also known as Latin-1

We need this for the old Desigo PX controllers!

ISO 8859-1 is also known as Latin-1
@kfenner
Copy link
Collaborator Author

kfenner commented Jul 6, 2017

Hey @fh1ch: How do we want to handle different encoding types when writing? In the JSON world, everything is UTF-8 so the clients don't care. But what happens when we want to write a value back to the device? We would need to convert the UTF-8 string to the original encoding. I already identified the methods I'd need to change, but it's not clear to me, how I even get to know which encoding to set.

@fh1ch
Copy link
Owner

fh1ch commented Jul 7, 2017

@kfenner first of all, thank you very much for this contribution. Since the supported set of encodings in Node.JS is quiet small, this stack doesn't support all the encodings specifiedby BACNET. I'm also not entitely sure if only the latin-1 subset is supported by BACNET as there is no '-1' postfix in the specs (see https://de.m.wikipedia.org/wiki/ISO_8859). However, I think we can give it a try...

LGTM 👍


Regarding the writing: I think we have to expose the encoding parameter as well when reading strings (simmilar to the encodin type when reading a property value). We simply pass it then to the write call and ensure that way full interoperability. Your thought on this?

Please add this to the additional festure list at #28

@fh1ch fh1ch merged commit 45136ca into master Jul 7, 2017
@kfenner
Copy link
Collaborator Author

kfenner commented Jul 7, 2017

Had some doubts too regarding ISO 8859-1 but BACnet spec states it explicitely: http://www.bacnet.org/Addenda/Add-135-2008k.pdf

image

@kfenner
Copy link
Collaborator Author

kfenner commented Jul 7, 2017

Regarding the writing: I checked in our C++ implementation and it is exactly like this: The encoding is retrieved and stored while reading and then later used when writing. I will do the changes...

@fh1ch fh1ch self-assigned this Jul 9, 2017
@fh1ch fh1ch added this to the 0.0.1-beta.9 milestone Jul 9, 2017
@fh1ch fh1ch deleted the fix/iso8859-string-decoding branch July 9, 2017 08:05
@fh1ch
Copy link
Owner

fh1ch commented Jul 9, 2017

I will do the changes...

@kfenner awesome, looking forward to it 👍

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

Successfully merging this pull request may close these issues.

2 participants