athombv / org.knx

KNX for Homey
0 stars 2 forks source link

Code enhancement: Use the KNX lib for Datapoint parsing #24

Open petero-dk opened 4 weeks ago

petero-dk commented 4 weeks ago

Right now the app uses its own simple datapoint parsing in: https://github.com/athombv/org.knx/blob/master/lib/DatapointTypeParser.js

However the knx lib actually have a very robust and detailed parsing library that is also extendable, I have used those methods for the receive telegram flow: https://github.com/athombv/org.knx/pull/21/files#diff-e07d531ac040ce3f40e0ce632ac2a059d7cd60f20e61f78268ac3be015b3b28fR58-R59

A first step would be to simply make DatapointTypeParser use that implementation instead of its own implementation, so we leverage any bug fixes in the upstream library.

What do you think?

ttherbrink commented 3 weeks ago

Personally i think that if there are no bug with the parser at the moment we should leave it be. It allows us to create quicker fixes than waiting for a library to merge our fix. If there are any significant improvements in using the KNX lib parser i am open to discussion on that matter.

petero-dk commented 3 weeks ago

I will not touch it for now, it was just an observation.