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

Enabling MQTT write: works, but values do not stick #6

Closed
webmaster-contentis opened this issue Feb 27, 2022 · 12 comments
Closed

Enabling MQTT write: works, but values do not stick #6

webmaster-contentis opened this issue Feb 27, 2022 · 12 comments

Comments

@webmaster-contentis
Copy link

webmaster-contentis commented Feb 27, 2022

I enabled MQTT write in the revpipyload configuration. Publishing a value of (for example) 1024 to /revpi000/set/PwmDutycycle_1 causes the PWM output to go to 1024 for one cycle only, then it falls back to 0.

I checked the logs: no errors. I checked that no other processes are active on /dev/PiControl0 and there were none, at least not on the UNIX level.

So writing works in principle, the value just gets reset to 0 in the next cycle.

After reading the code for revpypiload I traced the problem to revpipyload.py. The issue seems to be that it instantiates both an MQTT server and a something called the procimgserver. My theory is that if you enable MQTT writing (like I have) these both want to write the I/O ports and they end up overwriting each-other's value.

For discussion, please see https://revolutionpi.com/forum/viewtopic.php?f=6&t=3320&sid=144746f6c08d0eba79aa1c622869e6ff

Assuming that this is the problem: I am not sure there is a trivial solution. If I were to pass the instance of revpimodio2.RevPiModIO() that the procimgserver creates to the MQTT server, that might not have the correct configuration for the MQTT server, and vise versa.

If I were to make an instance of revpimodio2.RevPiModIO() and pass that into the __init__() of procimgserver and also mqttserver I end up exposing internal implementation details of both of these.

At this point, I am not even 100% sure that my analysis of the problem is correct. :-( Not sure how to proceed, please advise.

@webmaster-contentis
Copy link
Author

webmaster-contentis commented Feb 27, 2022

When I enable both send_on_event and write_outputs in /etc/revpipyload/revpipyload.conf, writing values via MQTT works.

That still leaves me confused about the logic of the configuration. Below is the original code. It seems that autorefresh is enabled only in the case of send_on_event being set. Is that how it should be?

            self._rpi = revpimodio2.RevPiModIO(
                autorefresh=self._send_events,
                monitoring=not self._write_outputs,
                configrsc=proginit.pargs.configrsc,
                procimg=proginit.pargs.procimg,
                replace_io_file=self._replace_ios,
                shared_procimg=True,
            )

I changed that to the code below, where autorefresh gets enabled when either send_on_event or write_outputs have been set. This seems to work.

            self._rpi = revpimodio2.RevPiModIO(
                autorefresh=self._send_events or self._write_outputs,
                monitoring=not self._write_outputs,
                configrsc=proginit.pargs.configrsc,
                procimg=proginit.pargs.procimg,
                replace_io_file=self._replace_ios,
                shared_procimg=True,
            )

@naruxde
Copy link
Owner

naruxde commented Feb 28, 2022

Are you using another RevPiModIO instance on the RevPi without the 'shared_procimg=True' flag? For example in the Shell or as control program? Your described behaviour will happen if another process is using the process image exclusively.

I tested the system with MQTT set and a parallel running 'PLC Monitor' in RevPiCommander. Everything works...

@webmaster-contentis
Copy link
Author

webmaster-contentis commented Mar 9, 2022

I have not tested with RevPiCommander, only revpipyload. You seem to be using a slighly different scenario than I am. No other instances of RevPiModIO running (I found out about already).

What is your config file, please?

@naruxde
Copy link
Owner

naruxde commented Mar 11, 2022

I'm so sorry!!! Now I understand!
In your fist configuration you did NOT enable send_on_event - And now I have the same problem!

You are absolutely right with your investigation! If you enable send_on_event everything will work because of the autorefresh value. Generally it is right, that autorefresh is not True, if you do not use send_on_event. That saves CPU resources on the device. In an older version of RevPiModIO2 with enabled shared_procimg the new value of an output was written instantly after assigning. This was not desired because it messed up the synchronicity of the cyclic programs. It was therefore reintroduced that the values must also be brought into the process image using the known write functions.

Unfortunately, the call is missing in mqttserver.py. Could you test this change with the original revpipyload version?

Index: revpipyload/mqttserver.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/revpipyload/mqttserver.py b/revpipyload/mqttserver.py
--- a/revpipyload/mqttserver.py	(revision e4ced5539ec4493d09d0bdc7e5b1192e8cb89bf7)
+++ b/revpipyload/mqttserver.py	(date 1646976056759)
@@ -333,6 +333,9 @@
                 # Write Value to RevPi
                 try:
                     io.value = value
+                    # Write data without autorefresh
+                    if not self._send_events:
+                        io._parentdevice.writeprocimg()
                 except Exception:
                     proginit.logger.error(
                         "could not write '{0}' to Output '{1}'"

Thank you for your patience with me!

@kjkoster
Copy link
Contributor

kjkoster commented Apr 5, 2022

I can confirm that your patch solves my problem. I just tested it by removing my own patch and adding yours instead.

Thank you for fixing this. If you have the time I would really like to see this get rolled into a release version, so I can stop running patched code on my systems.

@kjkoster
Copy link
Contributor

kjkoster commented Apr 5, 2022

(and yeah, I am webmaster-contentis, in case you are wondering).

@naruxde
Copy link
Owner

naruxde commented Apr 5, 2022

Okay, perfect! You can download the actual developer version here: http://revpimodio.org/dnl/revpipyload_0.9.6d-1_all.deb and install it via

sudo dpkg -i revpipyload_0.9.6d-1_all.deb

If we release the 0.9.7, your system will automatically update via apt-get update && apt-get dist-upgrade to 0.9.7.

@kjkoster
Copy link
Contributor

kjkoster commented Apr 5, 2022

What might be the time lines for an official release?

@naruxde
Copy link
Owner

naruxde commented Apr 5, 2022

This week 😃

And i believe Kunbus will put it to the contrib repository as soon as possible!

@kjkoster
Copy link
Contributor

kjkoster commented Apr 5, 2022

This week I can wait for the official release. :-)

@kjkoster
Copy link
Contributor

@naruxde Can you please roll this into a release? I'd like to switch back to running release code and undo the monkey patches. :-)

Is there anything I can do to help?

@kjkoster
Copy link
Contributor

This was rolled into a release and if you have this issue as I did, you can now solve this with 'apt update' followed by 'apt full-upgrade'.

Thanks to @naruxde and others to fix and release this.

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

3 participants