Closed nznobody closed 3 years ago
Dear Manu,
first of all, thanks a bunch for your excellent work on adding support for MicroPython to the VEDirect library. It is sweet that people will now be able to use it on microcontrollers running MicroPython.
With kind regards, Andreas.
PS: I ran black over the file, it changed the quotes. Shall I revert those?
It is fine, no worries.
With commit 5dd6da3e, you said "add threading async reader. Not recommended for use probably... ". Do you still believe we should bring this in, or shall we just add your contribution without this commit?
Hello Andreas,
Thanks so much for taking the time to add CI testing, this will definitely make adding to this repository much more streamlined in the future :)
Also, great that you are such an active maintainer and working to integrate my changes 😄
With specific regard to commit https://github.com/hiveeyes/terkin-datalogger/commit/5dd6da3e476801b7d47c2238ad1dbb22c7422536: I have tested it and it appears functional on a Fipy; however I am not sure about other Micropython and it is not well protected against errors in CPython. Possibly, if this feature is desired, I should polish it off a little. Additionally I have not researched the effects of threading on sleeping and power consumption.
I also did not use a lock and am unsure how threading will handle this in micropython. Since micropython lacks a more full threading library, I have not used it much.
Do you think it is worthwhile having threading? My rationale originally was because the VE devices often simply send out their values once every 10 or so seconds. That means if you synchronously wait for values you could be blocked for upto 10 seconds. I hope to read two VE devices and that means upto 20 seconds just waiting. Threading and storing the values resolves this.
I also have a minor change in there to insert the device product id (PID) into the variable name: "vedirect-{}:{}".format(product_id, key)
. This was to allow reading a couple different VE devices at once. However, upon reflection it may be better to use a name provided from the settings.py
since people may want to read two identical products (e.g. two MPPT controllers).
Do you think it is worthwhile me tidying up threading or removing it with these insights?
Thanks again for your time, Manu
Do you think it is worthwhile having threading? My rationale originally was because the VE devices often simply send out their values once every 10 or so seconds. That means if you synchronously wait for values you could be blocked for upto 10 seconds. I hope to read two VE devices and that means upto 20 seconds just waiting. Threading and storing the values resolves this.
Thanks for adding this explanation. It perfectly makes sense in this case. If there is really no way to send a signal to request a reading, I also don't see any other way to work around blocking the main thread.
I also have a minor change in there to insert the device product id (PID) into the variable name:
"vedirect-{}:{}".format(product_id, key)
. This was to allow reading a couple different VE devices at once. However, upon reflection it may be better to use a name provided from thesettings.py
since people may want to read two identical products (e.g. two MPPT controllers).
When the current flavor of parameter-naming might lead to a collision, I would welcome amending the logic to be able to read more than one device of the same kind, yes.
Hi again,
That means if you synchronously wait for values you could be blocked for upto 10 seconds.
If there is really no way to send a signal to request a reading, I also don't see any other way to work around blocking the main thread.
I am not into the details of the VEDirect protocol at all, so please have mercy. On a quick research, I found this on page 4 of [1]:
Message format
The device transmits blocks of data at 1 second intervals.
and that at [2]:
How often you send a HEX message doesn't matter: there is no minimum. But beware, after receiving a HEX message, the Victron product will stop sending its usual TEXT frames for a few seconds. Therefor, if you keep sending HEX messages, you'll never receive the TEXT frame any more. Which might matter in case you are depending on data in the TEXT frame.
Might this be even related to your observations in any way?
With kind regards, Andreas.
[1] https://www.sv-zanshin.com/r/manuals/victron-ve-direct-protocol.pdf [2] https://www.victronenergy.com/live/vedirect_protocol:faq#q7how_about_frequency_of_transmitting_hex_messages
Hello Andreas,
Having reviewed your material: I agree that threading will not be required. A 1 second wait seems acceptable. You are correct that longer periods were seen with HEX messages. However, they will not be likely in this scenario. I will roll back the threading changes, but leave in the name collision avoidance. I might not get time to do it this week and am away next week, but will do it when I return.
Thanks again for your rigorous assistance with these merge requests. Kind regards, Manu
Dear Manu,
thank you for your response. I got some minutes of spare time, removed 5dd6da3 but added c6a494d instead. If in the future we will need the async/threading feature, I am open to bring it in again.
With kind regards, Andreas.
ae720856b also fixes the installation of the improved VEDirect driver. Because the dependency_links
arg was dropped by pip since v19 (see also [1]), there is no way to add custom / Git-repository-based Python modules to the official install_requires
section in any way. So, I decided to put it into an "extras" section.
In this matter, the package can either be installed in development mode like python setup.py develop
or, when using pip
, like pip install --editable=.[vedirect] --find-links="https://github.com/nznobody/vedirect/tarball/345a688#egg=vedirect-2.0.0"
.
Without this amendment, it would break a vanilla, non-development installation attempt from PyPI using pip install terkin
.
[1] https://stackoverflow.com/questions/64482089/python-setup-py-dependency-as-url-to-tar-or-git
Dear Manu,
with 013ce38, I added a corresponding software test based on some testing infrastructure I found in the upstream vedirect
repository. Kudos again to @karioja, @jmfife and you.
With kind regards, Andreas.
Hi there,
based on my fork of @karioja's Python library for decoding the Victron Energy VE.Direct text protocol [1] (thanks a stack [2]), this patch will add support for MicroPython.
On top of that, it will now understand a booleanasync
setting parameter, which will divert the actual sensor reading to a dedicated thread.Cheers, Manu.
[1] https://github.com/nznobody/vedirect [2] Also thanks to @jmfife, who took over maintenance, added significant improvements and published the library on PyPI [3]. [3] https://pypi.org/project/vedirect-jmfife/
PS: I ran black over the file, it changed the quotes. Shall I revert those?