search

ned-kelly / docker-voltronic-homeassistant

Programmatically read data from your Voltronic, Axpert, Mppsolar PIP, Voltacon, Effekta etc Inverter and send it to Home Assistant via MQTT - Works with RS232 & USB!
GNU General Public License v3.0
260 stars 140 forks source link

performance optimization #52

Open kchiem opened 2 years ago

kchiem commented 2 years ago

move constant declarations outside the function calls, no need to re-run them for each call.

ned-kelly commented 2 years ago

@kchiem could you re merge this with the latest from master and i'll get this merged into prod!

kchiem commented 2 years ago

@kchiem could you re merge this with the latest from master and i'll get this merged into prod!

I've actually optimized it to run even faster. I'll re-do the PR.

kchiem commented 2 years ago

Hi Ned,

So I took a look at the new master and the pull request you merged in and have a question:

The HA mosquitto broker addon now requires a client id, The pull request you merged in handles this by defining a new field in the mqtt.json file and having all the related scripts pull that field to pass. Are there any other plans for using a specific client id like this? Otherwise, all that effort seems excessive. There's two other ways I can think of to handle this:

  1. Build the docker container from a later debian image than stretch (I'm using bullseye), which will pull in a newer version of the mosquitto-clients package and not require one to specifically provide the client id. This is the solution I went with, which is why my version of the scripts don't have to provide -i.

  2. The later version of mosquitto clients I'm using says the default for the client id is:

-i : id to use for this client. Defaults to mosquitto_pub_ appended with the process id.

Thus, if you want to stay on the current debian image, the scripts could also be modified to call the mosquitto clients with -i mosquitto_pub/sub_$$ to do the same thing as the later client versions.

My preference is obviously with option 1, or option 2 at worse. Why handle a specific client id as in your merge? Thoughts?

ned-kelly commented 2 years ago

I'm happy to change it over to Stretch if you've tested it? - I won't have time to test with a real inverter for another 1-2 weeks - what inverter are you running?

ned-kelly commented 2 years ago

Sorry mixup - I meant to say move it over to bullseye.

kchiem commented 2 years ago

I'm happy to change it over to Stretch if you've tested it?

Yeah, I've been running my container based off debian:bullseye for at least 2 weeks now.

kchiem commented 2 years ago

what inverter are you running?

I'm running the MPP Solar 2024LV-MK.

mohannaddubagh commented 2 years ago

Hello, first of all, thank you for sharing your projects. I tried hard to implement this project, but to no avail. I did not get any information from my device. I am a beginner in this field. Can you help? I wish there was a video explaining how to work. my solar inverter axpert vmiii 3500W Thank you for help