Community
OpenEnergyMonitor

OpenEnergyMonitor Community

Emonhub minor code cleanup, enhancements and python3 support

Tags: #<Tag:0x00007fc9b7794c70> #<Tag:0x00007fc9b7794a40>

Hello,

I have a branch of emonhub which I did some local development on a couple of years ago. It’d be good to get that merged if there’s appetite for it. It included:

. Works under python3 (python 2.7 support ends in two and half years or so)
. Interfacer which executes an arbitrary external command, and applies a regex to the output
. Interfacer
. Some minor code cleanups (some of which are necessary for the code to also be legal python3, and all of which follow the python coding recommendations)
. Support for restarting dead threads automatically
. systemd support and integration (watchdog restart)

If there’s appetite to get some or all of this merged, I’m happy to forward port this stuff and clean up the patches - the python3 changes could be enhanced so that it runs under either python 2 or 3 - but I wanted to check what’s likely to get merged before putting the work in…

Happy to structure and breakdown the work into individual pull requests of course…

Thoughts, opinions?

2 Likes

Hello @tim, that sounds great, thanks! in particular:

. Support for restarting dead threads automatically
. systemd support and integration (watchdog restart)

@pb66 and I have been discussing a change to the way the ‘message bus’ currently works in emonhub away from using pydispatcher, but it doesn’t sound like that would affect your proposed changes.

OK, that’s good to hear. Apart from the upcoming end of support, the main advantage is probably that many new libraries are python 3 only, and sooner or later I should think some new feature is going to be wanted (it’s possible to elect to not pull in that library at run time, and so disable the non-available functionality on python 2).

That was my original reason for porting to python 3, since I wanted to use a library which did https with keepalives for submitting to emoncms, to reduce network traffic.

python 2 code which is also valid python 3 tends to be clearer to read too IMO, and at some point you will probably want to make python 3 the default, and maybe later drop python 2 support (so that you can use python 3 only language features) - I’ll see if I can think of some way of getting it to run under python 2, but use python 3 if python 2 is unavailable…

I’ll implement the changes one at a time and submit pull requests, so lets see how it goes.

BTW, I’m assuming it’s OK to target Python 2.7 (released 7 years ago) as the oldest Python version? I haven’t tried, but I suspect that emonhub won’t run on 2.6 at the moment anyway, and the only Linux distro which people regularly run these days which includes Python 2.6 is RHEL 6.x (and derivatives - released in 2010). I doubt anyone is running emonhub on it, and even here you can install python 2.7 (or 3.x) from third party repos, or upgrade to RHEL 7.x (which ships Python 2.7 by default).

Nice! That could probably reduce the cost if you use a mobile internet connection with the Pi… :slight_smile:
I always wondered how much MB is consumed by just a few inputs…

Hello @TrystanLea @pb66 ,

I’ve done more work on the systemd support, and pushed the changes to my github repo.

I think using the systemd watchdog to restart the whole process is probably a better idea than restarting individual threads, since that keeps the code a bit simpler and is likely to be more reliable. New topic for that specific change here - RFC - systemd integration with watchdog support and restart on thread failure - I’d appreciate some testing, code review and feedback on this.

Tim.

Hi Tim,

I’ve been working away form home the last 3 days and not managed to find time to comment (here or on github), I did start writing this comment before I left so it originally replies to your original post, with some additional replies to your subsequent posts.

Support for systemd would be a great addition, interested to see your PR for this, did you retain the use of the /etc/default/emonhub config file? and the ability to create the logfile folder if it doesn’t exist? - - Ok, so I see your PR now and the immediate thing that sticks aside from the complexity is the change to how emonhub logs errors, emonhub.log is the most useful debugging tool we have, therefore for backward compatibility, ease of use, clearer logfiles and to keep all existing how-to’s and previous forum threads valid, not to mention the emoncms config module and those users that read/parse the emonhub.log file into something else (not good practice IMO, but I know some users have done it), emonhub.log must remain, at least as the default solution. The 2 logfile approach works much better as in normal running all the actual systemctl messages will be lost in the sheer quantity of normal emonhub.logs and get rotated out, so you are effectively swapping the emonhub.log to the systemd emonhub log and doing away with any systemd records, unless you avoid using INFO or DEBUG loglevels. Plus emonhub can run on windows too, so the default also needs to cater for that. If the user chooses to do away with emonhub.log, that could be an option, but IMO not the default.

Very interested to see whats required for running under Python3, the original reason emonHub (or rather it’s predecessor OEM Gateway) was written in Python2 primarily because the ConfogObj module wasn’t, at that time ported to Python3, that’s no longer the case. I guess a version that runs on 2 and 3 would be preferable to ease the transition (for both users and devs with very little Python3 experience like myself) so I would be interested in knowing how much extra work that would entail.

I also have a working solution for restarting the crashed threads, although it hasn’t been tested to any degree as I do not have an “emon-pi” variant emonhub in service, it would be great to compare notes so to speak. — Now I read that your solution is in the systemd service, which IMO isn’t the best place to be doing it. The solution I was working towards, restarts threads from within emonhub without any need to restart. This avoids interrupting other threads that are working fine, the planned reintroduction of buffering will also be impacted by doing it in the systemd as any buffered data will be lost on a restart.

The clean-ups would be great and coding standards should be enforced as PRs are submitted, perhaps some automated Travis checks are needed as on the emoncms repo to assist contributors, correcting the coding periodically in this way messes with the commit history, you are right it needs doing, but it would be better to get it done prior to merging the PR’s IMO and that is in the hands of the repo maintainers @glyn.hudson and @TrystanLea.

I am pretty busy this weekend as I am away again Mon AM but I will try to review in more depth, I just thought I should voice my opinion sooner rather than later on a couple of points, hopefully before you invest much more time in them, Having said that I have no control over the oem/emonhub repo so the changes may just get merged regardless of my opinion.

If I get time I would also like to write a post about emonhub “thread is dead” issues, because the “cause” (incomplete or poor coding) needs to be addressed more importantly than the “effect” (the thread dying). In short 3 things need to happen.

  1. Better sanitation of incoming data, more errors need catching and handling within emonhub. It is the responsibility of each interfacer to ensure it handles the data correctly and filters away anything that could cause a problem.
  2. The threads need to report a trace back before dying, this would give us the info required to understand the crash and implement further checks and error handling.
  3. Restart the thread from within emonhub, it’s quicker, simpler and less destructive than restarting emonhub entirely.

This is the ONLY way that the issue can be addressed because regardless of how you restart the thread, emonhub or the device, without addressing the “cause” it will only happen again and again, for a range individual reasons, but the complaint will always be the same “my emonhub keeps restarting for no apparent reason”.

Although posted here on your thread @tim, these comments about (not) restarting emonhub are also in response to other suggestions along the same lines and hopefully to take the focus away from restarting and on to avoiding the crashes in the first place, which will never happen if we automate the restarting without getting tracebacks.

Hi Paul,

Thanks for your feedback. I’ll respond to the python3 specific bits here and the others on the other thread.

I wouldn’t call myself a Python expert by any stretch, but all the recent development I’ve done is on Python3, when I did the conversion a couple of years ago for emonhub, the changes were relatively minor. I’ve also done some enhancements on a library which had python2 and python3 support. There are both forward and backwards compatibility shims which are pretty easy and un-intrusive. Probably best to just try a conversion on an up-to-date code base, and see what the patches look like…

I agree wholeheartedly - I’ve just been using pylint to check my own work (which I recommend - it’s caught more than one non-obvious bug in my code) - perhaps something like this?

If I get time, I’ll experiment with it a bit, maybe as a pre-commit hook? I’ve never set up travis, and have only done some minor work on git hooks for some private repos, but could also try and look at that if I have time.

I have also been having a problem with threads dying and have implemented thread traceback reporting and restarting in this pull request. Dunno if this is right approach but seems to do the job:

I would also agree that the threads should be monitored and restarted from within emonhub.

1 Like

@pb66 did you have any thoughts on my proposed changes above?

Sorry @beaylott I will take a look soon

1 Like

@TrystanLea @pb66 @tim Did anything come of this? Is emonhub on Python 3 and is systemd now supported?

Not as far as I know.

I would be happy to have a look at making the changes for Python 3. I have to say I was not totally convinced about the proposal to couple it into systemd in the code as that might cause compatibility issues with other init systems (which are still the norm on embedded devices) not to mention other operating systems. If a software watchdog is required the best thing would be to have a separate daemon process running to act as the watchdog. I could also look at this if there is interest.

1 Like

I agree. I do not like the idea of a systemd watchdog restarting emonhub because a single thread has crashed, as previously agreed that must be handled internally emonhub.

emonhub has no history of crashing the main thread so I doubt there is any call for a external watchdog.

I too would like to see what is involved for emonhub to move to Python 3 but it is tricky to weigh up properly when there is work to emonhub outstanding. I cannot guarantee I would be able to assist with emonhub issues once the change is made, I have zero python3 experience.

It is proving difficult to get a conversation going about emonhub moving forward an doing any more than just patching it up some more in it’s current form, which I don’t have much interest in doing.

I’m concerned the differences would will become to wide to ever reunite the different variants.

I have to admit I’m a little confused as to what was and wasn’t included previously because there have been more reported threads crashing and more logs posted with no tracebacks in. So I don’t know if stuff got left out or if it’s just not working as expected. I am also concerned that (as expected) the crashing has not been tackled and emonhub is just restarting threads silently. I have seen some of @borpin’s logs (some time ago now) that show the threads are continually restarting, but no clue as to why.

I have asked for some discussion on emonhub’s future role in the OEM project given that nearly all new devices coming on line are bypassing emonhub (emonESP etc) and anything other than relaying the RFM packets seems to get a recommendation to use node-red.

emonhub has no history of crashing the main thread so I doubt there is any call for a external watchdog.

Yeah I have not seen this either.

I have to admit I’m a little confused as to what was and wasn’t included previously because there have been more reported threads crashing and more logs posted with no tracebacks in.

All the ones I have seen (and I may have missed a few because I do not monitor these forums very closely) were from un-patched emonhub versions or unrelated issues. If there are issues I would encourage them to be reported on github so they can be dealt with more systematically. I also wanted the commit id to be posted somehow in the log file (at the top) so it was a bit clearer but I dont think that has happened yet.

I have asked for some discussion on emonhub’s future role in the OEM project given that nearly all new devices coming on line are bypassing emonhub (emonESP etc) and anything other than relaying the RFM packets seems to get a recommendation to use node-red.

Well as long as emonPi is the main hub it seems like there will be a need for it right? And it still seems to have a role for interacting with other systems (issues with interfacers not withstanding). It has been very useful for us where we want a small drop in application which can use to connect devices to emoncms. Also in container applications the emonhub container comes in at a few MB whereas node-red and/or emoncms is usually ~100MB. So I still see use cases.

Time to revive this thread on the basis that the Python2 deadline is fast approaching. Is there a plan to port emonhub to Python3?

https://www.ncsc.gov.uk/blog-post/time-to-shed-python-2

Here’s a link that doesn’t require Javascript: https://www.theinquirer.net/inquirer/news/3080802/ncsc-python-2-eol-warning-wannacry