Devirex commented 3 years ago

Please check if the PR fulfills these requirements
[x] Tests for the changes have been added / modified (needed for for bug fixes / features)
[x] commandref has been added / updated (needed for bug fixes / features)
[ ] CHANGED has been updated (needed for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) Feature to add MatchList-entries
What is the current behavior? (You can also link to an open issue here) Override complete MatchList
What is the new behavior (if this is a feature change)? Entries from myMatchList attribute will be added to default MatchList
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?) no
Other information:

sidey79 commented 3 years ago

Thanks you for that Pull Requests.

There is already a Attribut called

MatchList

There is not very much difference in the behaviour. Die you think we can reuse this and only change the evaluation ?

Devirex commented 3 years ago

Thanks you dir that Pull Requests.

There is already a Attribut called

MatchList

There is not very much difference in the behaviour. Die you think we can reuse this and only change the evaluation ?

I have changed myMatchList to MatchList but kept the beahvior. Some tests on my side showed that it is working quite good. If you press "deleteattr" all additional items are gone and the behavior is reverted to default MatchList.

codecov[bot] commented 3 years ago

Codecov Report

Merging #1026 (16c1dfc) into master (8aeae92) will increase coverage by 0.05%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1026      +/-   ##
==========================================
+ Coverage   59.78%   59.84%   +0.05%     
==========================================
  Files         111      111              
  Lines        8851     8851              
  Branches     1394     1394              
==========================================
+ Hits         5292     5297       +5     
+ Misses       2586     2577       -9     
- Partials      973      977       +4

Flag	Coverage Δ
fhem	`50.30% <100.00%> (+0.06%)`	:arrow_up:
modules	`59.84% <100.00%> (+0.05%)`	:arrow_up:
perl	`91.57% <ø> (ø)`
unittests	`59.84% <100.00%> (+0.05%)`	:arrow_up:

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

Impacted Files	Coverage Δ
t/FHEM/00_SIGNALduino/01_SIGNALduino_Attr.t	`100.00% <ø> (ø)`
FHEM/00_SIGNALduino.pm	`62.97% <100.00%> (+0.24%)`	: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 8aeae92...16c1dfc. Read the comment docs.

sidey79 commented 3 years ago

Looks good. I`ll check how we can extend automated tests.

sidey79 commented 3 years ago

I wrote some tests.

Currently i think there is a unexpected behaviour deleteing the attribute doesn't delete the attribute completly, only the content seems to be removed.

sidey79 commented 3 years ago

Do you need any assistance or need some more informations ?

sidey79 commented 3 years ago

I checked CommandDeleteAttr, and i think this does the delete job, so no need to delete the attribute inside SIGNALduino_Attr.

If you agree, i'll update my tests

sidey79 commented 2 years ago

@Devirex

Just a reminder for my last question

Devirex commented 2 years ago

@Devirex

Just a reminder for my last question

Hi,

sorry was on holiday the last three weeks. I’ll have a look at this, this week

Devirex commented 2 years ago

Hi,

had a look in this today, I don't understand where you find "CommandDeleteAttr". The "MatchList" Attribute was always be there and if there is no valid hash List which is set then it will default to standard.

sidey79 commented 2 years ago

Hi,

had a look in this today, I don't understand where you find "CommandDeleteAttr". The "MatchList" Attribute was always be there and if there is no valid hash List which is set then it will default to standard.

CommandDeleteAttr is a perl command and it already does the job of deleting a attribute so ther is no need, that we do it. https://github.com/fhem/fhem-mirror/blob/19f4992fb1cb47ee91e0f856de85ea12e23dfeb3/fhem/fhem.pl#L2380

RFD-FHEM / RFFHEM

Allow incremental addition of match list entries #1026

Codecov Report