volkszaehler / vzlogger

Logging utility for various meters & sensors
http://wiki.volkszaehler.org/software/controller/vzlogger
GNU General Public License v3.0
143 stars 124 forks source link

Fix libsml #29

Closed andig closed 9 years ago

andig commented 10 years ago

Aus der Mailing List:

On 05/25/2014 09:43 PM, Andreas Götz wrote:

Gibts jemanden der Kontakt zu TheCount hat um zu verstehen was bzgl libsml eigentlich "richtig" wäre? der Fork stammt von mir, behebt aber bei weitem nicht alle Fehler. Ihr könnt ihn gerne benutzen und mir auch Pull Requests schicken, aber:

  • der Fork erfordert einen C99-Compiler, während die alte Version (wahrscheinlich) mit C89 auskam,
  • der Fork ist wahrscheinlich nicht ABI-kompatibel zum Original (API passt glaube ich noch). Ich war auch zu faul, die Versionsnummer der Bibliothek entsprechend anzupassen. "Richtig" wäre, der libsml eine komplette Frischzellenkur zu verpassen (wenn man sie nicht gleich neu schreibt), insbesondere:
  • ein paar kleinere, aber wichtige Anpassungen der API,
  • komplettes Audit der Implementation; insbesondere die Fehlerbehandlung fehlt öfters,
  • vernünftige Build Comprehension – k.a. ob die libsml derzeit bspw. unter Windows kompilierbar ist,
  • verbesserte Dokumentation,
  • weitere Tests (und sich dabei überlegen, wie man das Unity-Framework vernünftig ins Repository einbindet). Mangels Zeit kann ich das leider nicht machen. Auf konkrete Anfrage erläutere ich die Punkte oben aber gerne näher, und auch auf Pull Requests werde ich reagieren. Viele Grüße Alexander
andig commented 9 years ago

@r00t- @mbehr1 in https://github.com/volkszaehler/vzlogger/issues/63 we can see that libsml does seem to have problems and outputs errors to the console. Any chance you could help with producing an improved version of that lib (error fix, open points above, merge of upstream fixes)?

r00t- commented 9 years ago

your post above is badly formatted and missing references. who is "TheCount"? which fork? who is "me"? first improvement: text is from this list message: http://volkszaehler.org/pipermail/volkszaehler-dev/2014-June/003659.html (but even there the context is missing)

we are talking about this obviously: https://github.com/dailab/libsml the code isn't the greatest, probably as mentioned above, but we did already get some fixes in there in the past...

andig commented 9 years ago

your post above is badly formatted and missing references.

As my post is 6 months old its probably not as bad is it appears.

mbehr1 commented 9 years ago

see #68

andig commented 9 years ago

@mbehr1 not sure you've had the time. Apart from #68, is there anything else from the todo list above worth looking into?

mbehr1 commented 9 years ago

I’ll take a look and see whether there is any issue with libsml usage/… (or e.g. see whether there are any further upstream changes).

Are there any other known problems from any logs? Any strange output from libsml?

Am 23.12.2014 um 12:06 schrieb andig notifications@github.com:

@mbehr1 https://github.com/mbehr1 not sure you've had the time. Apart from #68 https://github.com/volkszaehler/vzlogger/pull/68, is there anything else from the todo list above worth looking into?

— Reply to this email directly or view it on GitHub https://github.com/volkszaehler/vzlogger/issues/29#issuecomment-67941068.

Gruß

Matthias

mbehr1 commented 9 years ago

I took a look at all the changes from TheCount.

I think they are not urgently needed for vzlogger usage. The most interesting part is an updated unity test framework that adds memory leak checking. But the problems found seem to be only in the tests and not in the code used by vzlogger.

Does anybody know „TheCount“? Is he contributing to vzlogger or volkszaehler? In that case we might consider using his changes as the initial branch seems to be unmaintained (or at least unchanged since a long while).

Matthias

Am 23.12.2014 um 12:06 schrieb andig notifications@github.com:

@mbehr1 https://github.com/mbehr1 not sure you've had the time. Apart from #68 https://github.com/volkszaehler/vzlogger/pull/68, is there anything else from the todo list above worth looking into?

— Reply to this email directly or view it on GitHub https://github.com/volkszaehler/vzlogger/issues/29#issuecomment-67941068.

andig commented 9 years ago

Does anybody know „TheCount“?

This is all I know: https://github.com/TheCount/vzlogger

mbehr1 commented 9 years ago

Does not seem to be actively developed any longer. IMHO we dont need the changes. So I think we can close #29.

Gruß Matthias

Sent from a mobile device.

Am 23.12.2014 um 20:54 schrieb andig notifications@github.com:

Does anybody know „TheCount“?

This is all I know: https://github.com/TheCount/vzlogger

— Reply to this email directly or view it on GitHub.

andig commented 9 years ago

Thanks Matthias, much appreciated! You're my personal 'developer of the year' 2014!

mbehr1 commented 9 years ago

Thanks :-)

Gruß Matthias

Sent from a mobile device.

Am 25.12.2014 um 20:53 schrieb andig notifications@github.com:

Thanks Matthias, much appreciated! You're my personal 'developer of the year' 2014!

— Reply to this email directly or view it on GitHub.