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.

Sign up for GitHub

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

Jump to bottom

performance optimization #52

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

performance optimization #52

wants to merge 20 commits into from

Conversation

kchiem
Copy link

@kchiem kchiem commented Sep 29, 2021

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

@kchiem
move constant declarations outside the function calls, no need to re-run
9ffed7b 
them for each call.
@kchiem
- loop the subscriber process to handle mqtt server restarts
73aa986 
- reduce unnecessary wrapper processes
@kchiem
change temperature icon
7dd9283
@kchiem
update mqtt immediately after running a command
be2d786
@kchiem
- @nrm21 inspired changes--allows sending commands longer than 5 byte…
dbea469 
…s and

removes the need to provide qpiri/qpiws/qmod/qpigs reply sizes in the
config.
- also fixed some compiler warnings.
@kchiem
- @nrm21 inspired changes--allows sending commands longer than 5 byte…
5cb57c5 
…s and removes the need to provide qpiri/qpiws/qmod/qpigs reply sizes in the config.

- also fixed some compiler warnings.
@kchiem
Merge branch 'master' of github.com:kchiem/docker-voltronic-homeassis…
8d548e4 
…tant
@kchiem
fix typo
8c00a51
@kchiem
switch to while loops so that the output can be read in docker logs.
8b79ebb
@kchiem
- @nrm21 inspired changes--allows sending commands longer than 5 byte…
6d7e649 
…s and

removes the need to provide qpiri/qpiws/qmod/qpigs reply sizes in the
config.
- also fixed some compiler warnings.
@kchiem
Merge branch 'master' of github.com:kchiem/docker-voltronic-homeassis…
25c806c 
…tant
@kchiem
reduce the number of jq invocations drastically
6ffef39 
convert json output from the poller directly into a bash hash, so that
it can be iterated upon.
@kchiem
remove the push after running a command, didn't like the extra delay.
043d07e 
added publishing the poller's reply into mqtt.
@kchiem
don't need the fixed response sizes anymore.
25d9aaf
@kchiem
set retain flag on publishes, don't need to loop the init script
60186ea 
anymore with default HA install of Mosquitto broker using persistence.
@kchiem
add timestamps to stdout output (for docker logs)
4eef447 
retry up to 3 times total if the reply is anything under than "ACK"
@kchiem
updated for watch removal.
711370b
@ned-kelly
Copy link
Owner

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

@kchiem
Copy link
Author

kchiem commented Oct 6, 2021

@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
fixed a bug with response handling giving corrupt values.
c8b560d 
don't accept "NAK" results for QMOD, QPIGS, QPIRI, QPIWS queries on
polling.

clean up debug output a little.
@kchiem
Copy link
Author

kchiem commented Oct 7, 2021

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
Copy link
Owner

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
Copy link
Owner

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

@kchiem
Copy link
Author

kchiem commented Oct 11, 2021 •
edited
Loading

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
Copy link
Author

kchiem commented Oct 11, 2021

what inverter are you running?

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

@mohannaddubagh
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kchiem @ned-kelly @mohannaddubagh