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

libsml crc enhancements ? #103

Open
devZer0 opened this issue Dec 17, 2021 · 10 comments
Open

libsml crc enhancements ? #103

devZer0 opened this issue Dec 17, 2021 · 10 comments

Comments

@devZer0
Copy link

devZer0 commented Dec 17, 2021

please have a look at

devZer0/libsml-testing#12

maybe there is room for some enhancement of crc checks and error verbosity ?

@r00t-
Copy link
Collaborator

r00t- commented Jan 5, 2023

quoting @fkohlgrueber from devZer0/libsml-testing#12 (comment)

  • libsml doesn't check the checksum of transmissions (see here). That's the reason why the first (corrupt) message is processed by libsml.
  • Regarding corrupt files: It seems like libsml creates an empty file and then adds successfully parsed messages to it. That's why for the first file, it still reports the first message (101) and nothing else.
  • libsml's transport decoder expects escape codes to be aligned to 4 bytes (see here. If the alignment is off (e.g. due to missing bytes), the end of a file will not be detected (that's why the last two complete files aren't detected)

@r00t-
Copy link
Collaborator

r00t- commented Jan 5, 2023

wrote some code that at least detects the unaligned escapes and provides a warning: #112

it detects the issue in the file from devZer0/libsml-testing#12 right away:

$ ./examples/sml_server - <libsml-testing/EasyMeter_Q3A_A1064V1009.bin  >/dev/null 
libsml: warning: could not read the whole file
libsml: warning: unaligned end esc sequence found in data, stream desynced?
libsml: warning: unaligned (likely) start esc sequence found in data, stream desynced?
libsml: warning: unaligned end esc sequence found in data, stream desynced?
libsml: warning: unaligned (likely) start esc sequence found in data, stream desynced?
libsml: warning: unaligned end esc sequence found in data, stream desynced?
libsml: warning: unaligned (likely) start esc sequence found in data, stream desynced?
libsml: warning: unaligned end esc sequence found in data, stream desynced?
libsml: warning: unaligned (likely) start esc sequence found in data, stream desynced?

@r00t-
Copy link
Collaborator

r00t- commented Jan 5, 2023

@fkohlgrueber:
thanks for your analysis!
as you seem quite deep into sml and the libsml code already, do you see a chance of contributing code to address those issues?
(while "volkszaehler" is currenrtly hosting the library, we don't have much development resources)

@r00t-
Copy link
Collaborator

r00t- commented Jan 5, 2023

it's very unfortunate that the libsml (transport) api is very limited,
it provides no way to notify the caller of errors.
all we can do is log to stderr, return 0, or try to fix the problem ourselves.

we should maybe define a new API and deprecate the current sml_transport API.

we already defined that we signal EOF/error by returning 0 from sml_transport_read(): #51 ,
we could do that in case of stream parsing errors,
at least vzlogger would respond by re-opening (since volkszaehler/vzlogger#362 )

@fkohlgrueber
Copy link

@r00t- sry for the late reply, didn't have much time for OSS work lately.

I'm writing a library for parsing SML in Rust, see here: https://github.com/fkohlgrueber/sml-rs. I'm pretty happy with the transport API and the parser is coming together too. I'm not too comfortable writing C and I prefer to put my time and energy into sml-rs, but I could assist in case you have specific questions regarding sml (transport).

Another idea I've been thinking about is to implement a C API for sml-rs. This would allow libsml or vzlogger to use the library. I guess that there would be some effort required to get a Rust project integrated into the build process, but other than that I think this should work fine. What do you think? Would it make sense for libsml / vzlogger etc. to use the transport implementation of sml-rs?

@r00t-
Copy link
Collaborator

r00t- commented Jan 17, 2023

@fkohlgrueber:
thanks so far!
even if you're implementing a parser in a different language, that' surely a good thing, and we can probably benefit from any experience gained there and from comparative testing.

i'm not sure on the efficiency of linking rust code into a C/C++ application,
vzlogger is already surprisingly resource-heavy as a C++ application, given it's size.
and the target for libsml is typically a raspberry pi (via vzlogger) or even a ESP8266 microcontroller

@fkohlgrueber
Copy link

As Rust compiles to machine code (using LLVM), interoperability with C/C++ can be done using C FFI. The overhead of this small and given the data rate of typical smart meters (one message per second, message size ~300 bytes), efficiency shouldn't be a problem. Regarding efficiency in general: I've designed sml-rs with embedded devices in mind. It can be used without an allocator (only using statically allocated memory) and is therefore even suitable for small microcontrollers. I'm using it on an ESP32-C3 and it works great. I haven't compared the performance and resource usage of sml-rs vs. libsml, but I'm pretty sure that sml-rs is at least on par with libsml.

Also, I put quite some effort into the transport layer. It will only return correct transmissions that are complete and unaltered. If there are errors, they are surfaced in the API such that upper layers in the software can decide what to do with them. Also, it is designed such that it doesn't crash, no matter what input it is given.

@r00t-
Copy link
Collaborator

r00t- commented Feb 9, 2023

(@fkohlgrueber: i was refering to efficiency at build-time. vzlogger being C++ is already quite heavy, but it can still be compiled natively on a raspberry pi somewhat sensibly, but rust would probably make that impractical.)

@felixwrt
Copy link

felixwrt commented Feb 9, 2023

(using my new account from now on - but still the same guy as @fkohlgrueber)

Ah, I see. The Rust compiler indeed needs more resources than a C++ compiler as it's doing more work (specifically lifetime checking). I haven't compiled rust code natively on RPi, but now that you mention it I'd like to try it. I've used cross-compilation to compile Rust programs for my RPi 1 which worked fine, but I can see that that would be challenging for a project using both C++ and Rust.

@mbehr1
Copy link

mbehr1 commented Feb 11, 2023

@fkohlgrueber I like the rust idea! (this vzlogger/libsml c/c++ code is so error prone...)

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

5 participants