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

New elements for attribute hardware #1172

Merged
merged 5 commits into from
May 1, 2023
Merged

New elements for attribute hardware #1172

merged 5 commits into from
May 1, 2023

Conversation

sidey79
Copy link
Contributor

@sidey79 sidey79 commented Apr 29, 2023

  • 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)

Hardware for esp32 and esp8266 does not guarantee, that it will load the right firmware, because the hardware name is searched in the filename.

  • What is the new behavior (if this is a feature change)?

Hardware names without cc1101 have a suffix s which makes them uniqe
Hardware names are also adapted to be compatible with RFD-FHEM/SIGNALDuino#278

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

yes, the following hardware attribute are renamed
esp32 to esp32s
esp8266 to esp8266s
promini is removed and replaces with
promini8cc1101
promini16cc1101
promini8s
promini16s

  • Other information:

@codecov
Copy link

codecov bot commented Apr 29, 2023

Codecov Report

Merging #1172 (73e8bc4) into master (5b181ec) will decrease coverage by 0.58%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1172      +/-   ##
==========================================
- Coverage   67.57%   66.99%   -0.58%     
==========================================
  Files         135      134       -1     
  Lines        9812     9807       -5     
  Branches     1570     1570              
==========================================
- Hits         6630     6570      -60     
- Misses       1887     1942      +55     
  Partials     1295     1295              
Flag Coverage Δ
fhem 56.50% <100.00%> (-0.69%) ⬇️
modules 66.99% <100.00%> (-0.58%) ⬇️
perl 90.33% <ø> (ø)
unittests 66.99% <100.00%> (-0.58%) ⬇️

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

Impacted Files Coverage Δ
FHEM/00_SIGNALduino.pm 63.90% <100.00%> (-0.35%) ⬇️

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

- updated Hardwarelist
@elektron-bbs
Copy link
Contributor

Mhmm, muss das mit dem angehangen "s" unbedingt sein? Müssen dann alle User das Attribut ändern?

Außerdem gibt es noch etliche Differenzen zwischen Attribut, Dateiname und commandref. Das müsstest du dir nochmal genauer ansehen. Hier nur einige Beispiele:

nano328
SIGNALduino_nano328s_3.5.1.hex
nano328

ESP32s
SIGNALduino_esp32s_3.5.1.bin
ESP32d

ESP8266s
SIGNALduino_esp8266s_3.5.1.bin
ESP8266d

Vielleicht auch noch Groß-/Kleinschreibung vereinheitlichen, z.B. cc1101 vs. CC1101?

@sidey79
Copy link
Contributor Author

sidey79 commented Apr 30, 2023

Mhmm, muss das mit dem angehangen "s" unbedingt sein? Müssen dann alle User das Attribut ändern

Ich sehe da aktuell keine einfache Alternative um eine Firmware eindeutig zu identifizieren. Betrifft hauptsächlich die ESPs welche sich ohnehin derzeit nicht über das Modul updaten lassen.

Außerdem gibt es noch etliche Differenzen zwischen Attribut, Dateiname und commandref. Das müsstest du dir nochmal genauer ansehen. Hier nur einige Beispiele:

nano328
SIGNALduino_nano328s_3.5.1.hex
nano328

Da nano328 bereits eindeutig ist, können wir dies so belassen. Es erspart den Anwendern das Anpassen ihres Attributen.

ESP32s
SIGNALduino_esp32s_3.5.1.bin
ESP32d

Ich finde kein ESP32d.

ESP8266s
SIGNALduino_esp8266s_3.5.1.bin
ESP8266d

Vielleicht auch noch Groß-/Kleinschreibung vereinheitlichen, z.B. cc1101 vs. CC1101?

Das würde zu weiteren Änderungen der Attribute führen. Bei der Suche nach der Firmware spielt groß/kleinschreibung keine Rolle.

FHEM/00_SIGNALduino.pm Outdated Show resolved Hide resolved
FHEM/00_SIGNALduino.pm Outdated Show resolved Hide resolved
sidey79 and others added 2 commits April 30, 2023 19:40
@sidey79 sidey79 merged commit 9b28f96 into master May 1, 2023
@sidey79 sidey79 deleted the sidey/newHWAttr branch May 1, 2023 10:08
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.

3 participants