MERGED: emonHub 'emon-pi' V2.0.0 - fixing thread is dead issue

TrystanLea · 9 August 2017 13:54

Great @Dave, did you run sudo service emonhub restart at the end too?

beaylott · 9 August 2017 13:55

Ah i see what you mean. i think this following condition will mean there isn’t (even if there are duplicates in the config file)…

github.com

openenergymonitor/emonhub/blob/58a9e694265ba5ee8822cf956bd05690c5e459f0/src/emonhub.py#L240

    
      
                      # check init_settings against the file copy, if they are the same move on to the next
                      if self._interfacers[name].init_settings == settings['interfacers'][name]['init_settings']:
                          continue
              # Delete interfacers if setting changed or name is unlisted or Type is missing
              self._log.info("Deleting interfacer '%s' ", name)
              self._interfacers[name].stop = True
              del(self._interfacers[name])
          
          
for name, I in settings['interfacers'].iteritems():
              # If interfacer does not exist, create it
              if name not in self._interfacers:
                  try:
                      if not 'Type' in I:
                          continue
                      self._log.info("Creating " + I['Type'] + " '%s' ", name)
                      if I['Type'] in ('EmonModbusTcpInterfacer','EmonFroniusModbusTcpInterfacer') and not pymodbus_found :
                          self._log.error("Python module pymodbus not installed. unable to load modbus interfacer")
                      # This gets the class from the 'Type' string
                      interfacer = getattr(ehi, I['Type'])(name,**I['init_settings'])
                      interfacer.set(**I['runtimesettings'])
                      interfacer.init_settings = I['init_settings']

… there shouldn’t be any duplicates.

Dave · 9 August 2017 13:55

yep, all working as it should be

Dave · 9 August 2017 14:20

@stuart
I get the following error when running the script…
ERROR MainThread Unable to create 'SMASolar' interfacer: __i__() got an unexpected keyword argument 'readdcvalues'

Regards
Dave

stuart · 9 August 2017 15:31

Hi @Dave

Take the “readdcvalues” parameter out of the emonHUB configuration file.

The only valid parameter values are:

inverteraddress
inverterpincode
timeinverval
nodeid
packettrace

Dave · 9 August 2017 15:56

Thank You Stuart!
I don’t know how on earth “readdcvalues” got in the config fine.
It all working now
Have you thought any more on pulling data from multiable interters?

Regards
Dave

stuart · 10 August 2017 15:44

Its really difficult to do this without having a 2nd inverter to test the code against

Dave · 10 August 2017 16:09

Where in the country are you based? as I have a spare inverter lying around that I’m due to put on eBay.

Regards
Dave

stuart · 11 August 2017 08:30

I’m in the Midlands, although strangely I’m looking to buy an inverter for some more panels I’m putting on my garage roof!!

glyn.hudson · 11 August 2017 14:16

Still working reliable for me on a couple of systems.

Robert.Wall · 14 August 2017 10:10

7 posts were split to a new topic: Sunny Boy inverters for sale

TrystanLea · 14 August 2017 12:02

One week on now, no issues on our test systems.

@Dave @beaylott @stuart have you seen any issues?

We will move forward with merging this tomorrow or wednesday, if there are no issues.

stuart · 14 August 2017 12:34

I must admit i’ve not tried it - but happy to have it merged if you are okay with the testing.

Dave · 14 August 2017 14:15

Hi Trystan
I don’t believe Ive had any issues following the update and Pushover is telling me that it last crashed 7 days ago.

Regards
Dave

TrystanLea · 15 August 2017 16:10

Great to see!

glyn.hudson · 16 August 2017 16:20

Thanks for testing. PR has now been merging into default èmon-pi branch.

github.com/openenergymonitor/emonhub

Emon pi develop

openenergymonitor:emon-pi ← openenergymonitor:emon-pi-develop

opened 07:10PM - 05 Aug 17 UTC

TrystanLea

+223 -171

This pull request includes @beaylott's "Thread exception reporting, thread resta…rting, restart counting..." changes and a native message queue implementation outlined by @pb66 in response to threading issues in the emon-pi variant of emonhub. **Explanation of native message queue** In order to move data between the interfacers in emonhub e.g data received by the JeeLib interfacer and then published by the MQTT interfacer, emonhub has been using a python library called pydispatcher. I had thought that it was a neat solution, moving data between threads with the ease of use similar to MQTT. However @pb66 highlighted that there was an issue with the implementation, that the intention of one thread per interfacer was not being preserved, the MQTT interfacer in particular was actually running in the RFM2Pi thread! Paul then described a solution that would remove the need for the external pydispatcher library with a simple native message queue implementation. Here is my attempted summary (with references to the lines of code that implement it): 1) We start with the main thread (in emonhub.py) 2) The main thread starts a scan of each interfacer ([L143](https://github.com/openenergymonitor/emonhub/blob/native_message_queue/src/emonhub.py#L143)) 3) The main thread finds a pubchannel on the RFM2Pi interfacer called 'ToEmonCMS' ([L149](https://github.com/openenergymonitor/emonhub/blob/native_message_queue/src/emonhub.py#L149)) 4) The main thread reads and removes one item from the 'ToEmonCMS' queue on the RFM2Pi interfacer. ([L155](https://github.com/openenergymonitor/emonhub/blob/native_message_queue/src/emonhub.py#L155)) 5) The main thread iterates through each interfacer looking for subchannel entries called 'ToEmonCMS' ([L158-L162](https://github.com/openenergymonitor/emonhub/blob/native_message_queue/src/emonhub.py#L158)) 6) The main thread posts a copy of the item read from RFM2Pi into the 'ToEmonCMS' queue on each subscribing interfacer. ([L168](https://github.com/openenergymonitor/emonhub/blob/native_message_queue/src/emonhub.py#L168)) 7) The subscribing interfacer then reads and acts on the queue in its own time and in its own thread. 8) The main thread then goes on to the next interfacer checking its pubchannel's, repeating steps 4 to 6 above. A single item is read at a time in order to avoid the case where a runaway interfacer monopolizes the main thread which could block the other interfacers, it also allows for the simple use of pop and append. The implemented result in this pull request fixes the threading issue that was present in the pydispatcher based implementation while reducing emonhub's dependencies, thanks to @pb66 for the idea and description of how to implement.

Update will be available to all emonPi’s via ‘emonPi Update’ via the web interface.

This new version is V2.0.0:

Those testing running the dev branch should change back to the emon-pi branch and pull in the changes.

borpin · 19 August 2017 14:28

Is this specific to the emon-pi or is it general as well? If so can we ditch a specific Emon-pi version; it just confuses anyone following on.

pb66 · 24 October 2017 12:58

See EmonPi Crash and EmonHub still dying?

pb66 · 26 October 2017 10:37

See emonTxV3 stops and requires reboot (emonhub) for issues relating to the http buffering

Further to the points I raise in that linked thread (send size throttling, max buffer size, log message text/positions and over-logging), the following issues also need to be tackled with the http buffering implementation.

“if int(self._settings[‘senddata’]):” cannot be where it is currently as this switch doesn’t directly control whether the data is sent, it controls whether the data is passed to the interfacer, the data must be consumed, the interfacer cannot just say “no I do not want the data” because that leaves it backing up on the queue using up ram.
The call to bulk post cannot reside inside the loop that adds the data to the buffer, if data is not regular the data in the buffer might never be sent if the network comes up after the last frame is put on the buffer.
The sendinterval switch cannot stay where it is as it allows the interfacer to sit idle between sends and only when the interval has passed will it processes the queue into the buffer before trying to send.
There are a lot of ifs and buts that prevent this code from doing anything at all unless all conditions met and then it does everything as one big packet, it empties the whole queue into the buffer, it then copies the whole buffer into a request and sends it, if it succeeds it deletes all of the “buffered” data directly from the queue, if it fails it will not retry again for 30secs and when it does retry it restarts from the beginning with an empty buffer, reloading every frame from the queue to the buffer, then to the request, this really isn’t the best way to do things.

These are just the “crucial” bits to make it work, there are also a number of things that are just not good practice, advisable or needed, mainly where inheritance and sub-classing has been ignored in favour of rewriting common code in multiple places, The biggest offender here is pretty much the whole action method which is most of the interfacer, most of this code should be in the run method of the basic interfacer and not over-ridden. The rest should be in the send method and most of whats in the send method belongs in the action method so that the data is sent regardless of whether/when any new data gets added to the buffer. The actual sending of the http request and the return of the response belongs in a separate method that belongs to a generic http interfacer class that this interfacer sub-classes, this is so that a (for example) pvoutput interfacer could also sub-class the generic http class and only the action method would be different as that is where the buffer gets parsed into a url and that generic “url_send_method” would get called and the raw reply passed back for that same action method to handle.

Just for the record, the action method is also where an independent and regular call to the emoncms/feed/fetch api could be made and that payload of feed values passed back into emonhub (remember why we made emonhub bidirectional and mulichannel) on a different channel that (for example) the MQTT interfacer is listening to, so that the data is published to the topic of your choice.

BUT, the mqtt development is blocked by the same things as this interfacer, it is too “bespoke” and written with an extremely narrow scope, plus the naming is no good, only a generic MQTT interfacer should be called mqttinterfacer, the bespoke emoncms mqtt interfacer should have been named emoncmsmqttinterfacer and the methods for subscribing and publishing should reside in the generic mqtt interfacer.

Getting processed feeds data from emoncms(org) on to mqtt should be as easy as defining a list of feed ids, an interval, a topic and a couple of other settings eg single mqtt topic or per value