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

additional sensors protocol 115 for Bresser 6-in-1 Comfort Wetter Center #1085

Merged
merged 10 commits into from
Apr 11, 2022

Conversation

elektron-bbs
Copy link
Contributor

@elektron-bbs elektron-bbs commented Mar 12, 2022

  • Please check if the PR fulfills these requirements
  • Tests for the changes have been added / modified (needed for for bug fixes / features)
  • commandref has been added / updated (needed for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
  • Bugfix (please link issue)
  • Feature enhancement
  • Documentation update
  • Unittest enhancement
  • other
  • What is the current behavior?
    (You can also link to an open issue here, if this describes the current behavior)
    No correct evaluation of the additional sensors.

  • What is the new behavior (if this is a feature change)?
    fixed packet length mode 18 byte
    new ground/earth sensor for humidity and temperature, indoor sensor for humidity and temperature

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    no

  • Other information:

elektron-bbs and others added 4 commits March 12, 2022 17:04
SD_ProtocolData.pm - fixed packet length 18 byte
14_SD_WS.pm - new ground/earth sensor for humidity and temperature, indoor sensor for humidity and temperature
@codecov
Copy link

codecov bot commented Mar 12, 2022

Codecov Report

Merging #1085 (465c05a) into master (1a8e7e7) will increase coverage by 0.19%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #1085      +/-   ##
==========================================
+ Coverage   63.66%   63.85%   +0.19%     
==========================================
  Files         130      133       +3     
  Lines        9494     9539      +45     
  Branches     1505     1513       +8     
==========================================
+ Hits         6044     6091      +47     
+ Misses       2288     2277      -11     
- Partials     1162     1171       +9     
Flag Coverage Δ
fhem 55.75% <50.00%> (+0.25%) ⬆️
modules 63.85% <50.00%> (+0.19%) ⬆️
perl 90.59% <ø> (+0.21%) ⬆️
unittests 63.85% <50.00%> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
FHEM/lib/SD_ProtocolData.pm 100.00% <ø> (ø)
FHEM/14_SD_WS.pm 65.59% <50.00%> (+0.05%) ⬆️
t/FHEM/14_SD_WS07/09_parseDatat.t
t/FHEM/10_SD_GT/00_load.t 100.00% <0.00%> (ø)
t/FHEM/14_SD_BELL/00_load.t 100.00% <0.00%> (ø)
t/FHEM/14_BresserTemeo/09_parseDatat.t 80.00% <0.00%> (ø)
t/FHEM/14_SD_BELL/09_parseDatat.t 80.00% <0.00%> (ø)
FHEM/lib/SD_Protocols.pm 79.56% <0.00%> (+0.10%) ⬆️
FHEM/00_SIGNALduino.pm 64.19% <0.00%> (+0.44%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Contributor

@sidey79 sidey79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haben wir dafür Testdaten?

new ground/earth sensor for humidity

@elektron-bbs
Copy link
Contributor Author

Bisher nicht,
Ich dachte, 2 oder 3 Nachrichten pro Protokoll reichen dir. Jetzt hast du noch je eine zusätzlich für den "Bresser_6in1 Thermo-/hygro sensor" und "Bresser Explore Scientific SM60020 Soil moisture Sensor".

@sidey79
Copy link
Contributor

sidey79 commented Mar 20, 2022

In der Pipeline befindet sich ein Fehler. Daher funktionioniert version_replace nicht so wie erhofft

@@ -1,4 +1,4 @@
# $Id: 14_SD_WS.pm 21666 2022-01-30 10:19:30Z elektron-bbs $
# $Id: 14_SD_WS.pm v3.5.4 2022-01-30 10:19:30Z elektron-bbs $
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich weiß nicht, ob das gut ist, in den Modulen die Versionsnummer zu ändern.
Die Ausgabe von "version" in FHEM kommt damit offensichtlich nicht so richtig klar:

00_FBAHAHTTP.pm        25767 2022-03-03 18:52:56Z rudolfkoenig
10_FBDECT.pm           25069 2021-10-13 05:58:51Z rudolfkoenig
14_SD_WS.pm            21666 2022-01-30 10:19:30Z elektron-bbs
# $Id: 00_SIGNALduino.pm v3.5.4 2022-03-12 17:55:30Z sidey79 $
# $Id: SD_ProtocolData.pm 3.5.x 2022-01-30 10:19:30Z elektron-bbs $

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was willst Du stattdessen dort hinterlegen?

Die SVN commit ID ist ja falsch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entweder gar nichts daran ändern, oder vielleicht statt "v3.5.4" nur "354".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das soll ja den Versionsstand der Datei darstellen.
Es bei der SVN commit ID zu belassen ist dadurch falsch.

354 ist ebenfalls eine gültige SVN Referenzen.

Vielleicht könnten wir einen Patch für die Anzeige vorschlagen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So wichtig erscheint mir das nicht. Mir ist eben bisher nur aufgefallen, das wir da aus der Reihe tanzen.
@HomeAutoUser und ich hatten irgendwann mal angefangen bei 0. Ich habe dann dort ab und an mal hochgezählt:

14_SD_AS.pm              350 2020-10-01 20:16:11Z elektron-bbs
14_SD_BELL.pm              0 2021-07-12 22:30:35Z HomeAuto_User
10_SD_GT.pm                1 2020-05-25 21:00:00Z elektron-bbs
88_SIGNALduino_TOOL.pm     0 2021-10-12 07:58:00Z HomeAuto_User

Anhand der relativ kleinen Zahlen erkennen wir, das die Dateien nicht aus SVN stammen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/fhem/fhem-mirror/blob/6bccfb00851718c0c70cf5df5b0a0ad024f0f9be/fhem/FHEM/98_version.pm#L67

Wir müssten hier nur eine Änderung der wirken, dass auch Werte <> Zahlen an der betreffenden Stelle erlaubt sind

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich habe die Regex angepasst

https://regex101.com/r/HUdUmu/1

Ich werde mal testen wie es damit aussieht

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ergebnis sieht ganz gut aus für den Anfang,

Patch ist eingereicht:
https://forum.fhem.de/index.php?topic=126908.new#new

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das zieht sich jetzt schon seit über 3 Wochen hin. Wollen wir nicht erstmal den Pull request übernehmen?
Ich hätte mittlerweile weitere Änderungen für das Modul in petto.
Die Anpassung der Versionsnummer kannst du ja auch später noch vornehmen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elektron-bbs

Wir können den PR so mergen aus meiner Sicht, die nicht optimale Anzeige des version Befehls verschwindet, sobald der Patch angenommen wurde.

@elektron-bbs elektron-bbs merged commit 923fbfd into master Apr 11, 2022
Devirex pushed a commit to Devirex/RFFHEM that referenced this pull request Apr 22, 2022
…ter (RFD-FHEM#1085)

SD_ProtocolData.pm - fixed packet length 18 byte
14_SD_WS.pm - new ground/earth sensor for humidity and temperature, indoor sensor for humidity and temperature
Devirex pushed a commit to Devirex/RFFHEM that referenced this pull request Apr 22, 2022
…ter (RFD-FHEM#1085)

SD_ProtocolData.pm - fixed packet length 18 byte
14_SD_WS.pm - new ground/earth sensor for humidity and temperature, indoor sensor for humidity and temperature
@elektron-bbs elektron-bbs deleted the master_Bresser-6in1_update branch May 23, 2022 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants