Closed iconnor closed 3 years ago
Hello @iconnor! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
Merging #32 (ead497c) into master (4294158) will increase coverage by
0.06%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #32 +/- ##
==========================================
+ Coverage 95.77% 95.83% +0.06%
==========================================
Files 6 6
Lines 828 841 +13
==========================================
+ Hits 793 806 +13
Misses 35 35
Impacted Files | Coverage Δ | |
---|---|---|
modbus4mqtt/modbus4mqtt.py | 94.50% <100.00%> (+0.38%) |
:arrow_up: |
tests/test_mqtt.py | 97.40% <0.00%> (-1.16%) |
:arrow_down: |
tests/test_modbus.py | 97.14% <0.00%> (+2.28%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 4294158...ead497c. Read the comment docs.
You set a pretty high bar - using the pep8 tool, it does mean new PRs have to clean up any code in files they touch. I am not against it, just letting you know why the PR kind of exploded in scope.
Goal: follow
mosquitto_sub --help
as closely as possible.Naturally, mosquitto_sub have different options and will negotiate SSL for you. Paho expects you to call tls_set. So, I borrowed the insecure flag from mosquitto_sub to call if this is on or off.
As you know, I am using let's encrypt, so I don't need to pass the ca_certs to get it all working. I left the option if you want but have not tested it. If you prefer, I can remove those and leave the
insecure
option as the only one.