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

mqttdaemon mode #44

Merged
merged 23 commits into from
Apr 6, 2021
Merged

mqttdaemon mode #44

merged 23 commits into from
Apr 6, 2021

Conversation

segaura
Copy link

@segaura segaura commented Mar 7, 2021

#39
closes #40

Hallo.
This pull request adds mqttdaemon mode.
It does this on top of previous proposed improvements, like the log revision.

I'm not particularly happy about three things that I hope can be improved by collaboration:

  • modularization, because mqttdaemon has been added in pyHPSU.py trying to keep where possible the previous code structure
  • reliability, because mqttdaemon and automatic mode both start threads and I have not checked new and existing code for thread safeness when they work together
  • testing, because I do not use mysql, hpsud, elm, backup, restore, ... and I can't say if something has been broken
  • I accidentally changed EOL characters of etc/pyHPSU/commands_hpsu.json and the results it shows completely changed in comparisons ...maybe a clear DOS vs UNIX EOL strategy has to be adopted, e.g. every contributor using text=auto in .gitattributes

Please ask me anything if something is not clear in the code.

@m-reuter
Copy link

in your Readme it should be
--mqtt_daemon
in the command examples (not --mqttdaemon )

Other than that, it is working nicely for me. Even got it working inside a docker. Thanks for the work!

@segaura
Copy link
Author

segaura commented Mar 22, 2021

Thank you, I corrected it, I'll push it when I find time to clean some other problems, like the EOL in hpsu_commands.json.

Please consider contributing your Dockerfile and the doker run / docker-compose example, it'll be of great value.

@pdcemulator
Copy link

We have a working dockerfile. Please consider to submit a PR to https://github.com/pdcemulator/pyHPSU-docker

@segaura
Copy link
Author

segaura commented Mar 22, 2021

Given pyHPSU uses install.sh to place only project's needed files in the user filesystem, would not be a good idea to include directly here the Dockerfiles?
It does not harm the rest of the project, I mean.

@m-reuter
Copy link

m-reuter commented Mar 22, 2021

sure, still experimenting which base image to use for docker. also plan to create an add-on for homeassistant, that way it could be run, configured and re-started from the web interface directly (for people using home assistant of course) - that would be a separate repo.
Also I agree with @segaura that it would be better to have a dockerfile in pyhpsu directly that way it can be maintained when new features and new library dependencies get added and it will be easier for users to find.

@segaura
Copy link
Author

segaura commented Mar 26, 2021

With latest command I reverted commands_hpsu.json EOL format to DOS, so it can be compared easily with the original one.

Hope I'll be able to find some time to tidy up code, also.

@segaura segaura mentioned this pull request Mar 26, 2021
@Spanni26
Copy link
Owner

Spanni26 commented Mar 27, 2021 via email

@segaura
Copy link
Author

segaura commented Mar 27, 2021

Why back to dos EOL? I moved all files to unix EOL si editing under unix is easier without all the ^M at the end of the line. Am 26.03.2021 um 19:23 schrieb segaura:

With latest command I reverted commands_hpsu.json EOL format to DOS, so it can be compared easily with the original one. Hope I'll be able to find some time to tidy up code, also. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2I5HGOTRAGQ5ZKPU42W23TFTGIZANCNFSM4YXW7SZQ.

I made it to make the pool request auditable, viceversa the comparison against the same file in Spanni26:testing was not possible (I changed t_room and added a few other new commands on the bottom).
I agree that getting rid of carriage return is a good thing, and including all files and agreeing on a .gitattributes approach, but I thought we can easily think of this later ...I can take care of that after we agree on a shared solution.

By the way, I am using Mqtt Daemon and Automatic Mode with MQTT Output as a systemd service since a month or so and it very stable, to my own surprise given I have not thought a lot of thread safeness when coding.

@segaura
Copy link
Author

segaura commented Apr 5, 2021

I did a first cleanup in the code.

It mainly rotates around cli argument parsing (complete refactoring) and config file parsing (minimum refactoring, so far).

For argument parsing I moved to argparse which is very powerful and compact, and enabled many things:

  • single "options" object containing all argument details, using it globally enable to get rid of various verbose, driver, lg_code, ...
  • automatic variable instantiation
  • automatic uppercase
  • default management
  • allowed argument values
  • error management
  • automatic mutual exclusion management, e.g. --backup/--restore
  • automatic real help
  • automatic version argument

This way -h/--help have argparse default behavious, showing an output like this

$ pyHPSU.py --wrong_command
usage: pyHPSU.py [-h] [--dictionary] [--version] [-a] [-f CONF_FILE]
                 [-b BACKUP_FILE | -r RESTORE_FILE] [-g LOG_FILE]
                 [--log_level {DEBUG,INFO,WARNING,ERROR,CRITICAL}]
                 [-l {EN,IT,DE}] [-d DRIVER] [-c CMD]
                 [-o {JSON,CSV,BACKUP,MQTT,FHEM,PDF,MYSQL,EMONCMS,INFLUXDB,HOMEMATIC,OPENHAB}]
                 [-v VERBOSE] [-p PORT] [--mqtt_daemon]
pyHPSU.py: error: unrecognized arguments: --wrong_command

or this

$ pyHPSU.py -h
usage: pyHPSU.py [-h] [--dictionary] [--version] [-a] [-f CONF_FILE]
                 [-b BACKUP_FILE | -r RESTORE_FILE] [-g LOG_FILE]
                 [--log_level {DEBUG,INFO,WARNING,ERROR,CRITICAL}]
                 [-l {EN,IT,DE}] [-d DRIVER] [-c CMD]
                 [-o {JSON,CSV,BACKUP,MQTT,FHEM,PDF,MYSQL,EMONCMS,INFLUXDB,HOMEMATIC,OPENHAB}]
                 [-v VERBOSE] [-p PORT] [--mqtt_daemon]

pyHPSU is a set of python scripts and other files to read and modify the values
of the Rotex® HPSU (possibly) also identical heating pumps from Daikin®).

optional arguments:
  -h, --help            show this help message and exit
  --dictionary          show complete command dictionary or specific command
                        help
  --version             show program's version number and exit
  -a, --auto            do automatic queries
  -f CONF_FILE, --config CONF_FILE
                        Configfile, overrides given commandline arguments
  -b BACKUP_FILE, --backup BACKUP_FILE
                        backup configurable settings to file [filename]
  -r RESTORE_FILE, --restore RESTORE_FILE
                        restore HPSU settings from file [filename]
  -g LOG_FILE, --log LOG_FILE
                        set the log to file [filename]
  --log_level {DEBUG,INFO,WARNING,ERROR,CRITICAL}
                        set the log level to [DEBUG, INFO, WARNING, ERROR,
                        CRITICAL]
  -l {EN,IT,DE}, --language {EN,IT,DE}
                        set the language to use [EN IT DE], default is "EN"
  -d DRIVER, --driver DRIVER
                        driver name: [ELM327, PYCAN, EMU, HPSUD], Default:
                        PYCAN
  -c CMD, --cmd CMD     command: [see commands dictionary]
  -o {JSON,CSV,BACKUP,MQTT,FHEM,PDF,MYSQL,EMONCMS,INFLUXDB,HOMEMATIC,OPENHAB}, --output_type {JSON,CSV,BACKUP,MQTT,FHEM,PDF,MYSQL,EMONCMS,INFLUXDB,HOMEMATIC,OPENHAB}
                        output type: [JSON, CSV, BACKUP, MQTT, FHEM, PDF,
                        MYSQL, EMONCMS, INFLUXDB, HOMEMATIC, OPENHAB] default
                        JSON
  -v VERBOSE, --verbose VERBOSE
                        verbosity: [1, 2] default 1
  -p PORT, --port PORT  port (eg COM or /dev/tty*, only for ELM327 driver)
  --mqtt_daemon         set up an mqtt daemon that subscribe to a command
                        topic and executes received command on HPSU

- If no command is specified, all commands in dictionary are executed
- To set a value use <command>:<value>

to accomodate this "-h" behaviour I am proposing to move the old "-h" functionality to a new --dictionary argument (it is possible to leave things like before, using a less standard approach, but let's discuss).

Other changes are:

  • a unique mqtt client name for mqttdaemon (subscribe), allowing to have more than one pyHPSU instance active simultaneously, through a unique per pyHPSU execution randomized value reusable globally where needed
  • HPSU.py, translations not present in dictionary listing managed with if instead of try-exception
  • commands_hpsu.json, fixed some command bytes as per issue t_room not working #40 and command list cleanup #41, added commands comp_active, pump_active, temperature_spread_pwm, offset_t_flow --> SHORT AND LONG DESCRIPTIONS STILL MISSING
  • fixed, mqtt_brokerport not used in mqttdaemon mode
  • fixed, removed maaany unused variables and import, not only those substituted with options (driver, port, verbose, ...) resulting in more that 100 lines of code removed

And two additional that are "value type" related:

  • canpi.py, management of command with "value type" and value of more than one byte
  • read_can(), desc field added to JSON output and JSON publish in mqttdaemon mode for commands with "value type" where `resp has valid decoding --> IMPLEMENTED AS A PROPOSAL ONLY FOR JSON OUTPUT WHERE AN ADDITIONAL FIELD DOES NOT HARM A LOT, TO BE DISCUSSED HOW OTHER PLUGINS HAVE TO BEHAVE

@Spanni26 Spanni26 merged commit 225d612 into Spanni26:testing Apr 6, 2021
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

Successfully merging this pull request may close these issues.

4 participants