add adapter webcal

[dirkhe]

Please add new Adapter to ioBroker.

Test diskussion in forum https://forum.iobroker.net/topic/62479/test-neuer-adapter-webcal

[mcm1957]

Link to adapter: https://github.com/dirkhe/ioBroker.webcal

[mcm1957]

@dirkhe
Looks like you changes from main to master. Within io-package.jsosn links refer still to main.
"extIcon": "https://raw.githubusercontent.com/dirkhe/ioBroker.webcal/**main**/admin/webcal.png",
"readme": "https://github.com/dirkhe/ioBroker.webcal/blob/**main**/README.md",

[dirkhe]

@mcm1957
Yes, you are right. I fixed some further things.
I guess here, the checker should be adapted, because I use this.setInterval, which means adapter.setInterval.
And here I understand, that the adapther himself store the intervalID, because of this comment:
/** Creates an interval that can automatically be cleared when the adapter is terminated */

[W504] setInterval found in "build/main.js", but no clearInterval detected
[W505] setTimeout found in "build/main.js", but no clearTimeout detected

BTW: Bluefox is invited

[mcm1957]

I guess here, the checker should be adapted, because I use this.setInterval, which means adapter.setInterval. And here I understand, that the adapther himself store the intervalID, because of this comment: /** Creates an interval that can automatically be cleared when the adapter is terminated */

[W504] setInterval found in "build/main.js", but no clearInterval detected [W505] setTimeout found in "build/main.js", but no clearTimeout detected

Its correct that the cleanup code is technically not required. But @Apollon77 statefd lately that's good praxis to cleanup anyway. I cannot decide if this is recommended or required when using adapter.setTimer/this.setTimer. I suggest to wait for Apollon77's official review.

[dirkhe]

In a earlier Version the clearTimeout was in, but than I saw the internal adapter setTimeout and thought this is better Variant and removed the manually clearTimeout. So for me it is ok, but than I would no longer used the adapter. setTimeout, because doubled clearTimeout is not needed.
bluefoy is maintainer now and read-write access, so I thing the "must be fixed" could be removed.

[mcm1957]

I thing the "must be fixed" could be removed.

Will be done by Apollon77 when he has time for review.

[mcm1957]

First of all - THANK YOU for the time and effort you spend to maintain this adapter.

I would like to give some (updated but preliminary) feedback based on my personal oppinion. This is NOT an offical review and @Apollon77 might have several additional suggestions or even a different oppinion to one or the other statement. Please feel free to contact him (or wait for a response from him) if you cannot follow my suggestsions or before you spend major effort.

• errors reported by adapter checker must be fixed

  Especially warnings
  '[W504] setInterval found in "build/main.js", but no clearInterval detected'
  '[W505] setTimeout found in "build/main.js", but no clearTimeout detected'

  It's OK and recommeded to use this.setTimerout / this.setInterval. And you are right, thos routines are designed to do internal cleanup at exit. But it's good praxis and recommended to do own housekeeping anyway and to use this.clearXXX if possible. Feel free to discuss this with @Apollon77, my recommendation is to use thissetTimeout/this.setInterval AND to use this.clearInterval/this.clearTime at onUnload code.

• reevaluate state roles

  Only the values specified here (https://github.com/ioBroker/ioBroker.docs/blob/master/docs/en/dev/stateroles.md) may be used as state roles. Do not invent new names as this might disturb the functionalyity of other adapters or vis.

  In addition the roles MUST match the read/write functionality. As an example a role value.* requires that the state is a readable state. Input states (write only) should have roles like level.*. Please read the description carefully. States with role 'button' should (must) have attribute 'common.read=false' set. A button ( "Taster" in german only triggers when you press but else has no state), so it should also be not readable. This is also like HomeMatic does it. A switch has clear states true or false and so can be read.

  I.e. 'fetchCal' is a button (role: button). So this state must have read attribute set to false.
  https://github.com/dirkhe/ioBroker.webcal/blob/1abacf1afe0e1f068228b3c1995fdbfddb550c85/io-package.json#L201

• check and limit intervals used to access cloud services

  Limit all intervals used to access cloud services to a reasonable value. If users configure an inapprobiate access rate this could lead to suspending the service by the provider for all ioBroker users. Please check whether a minimum polling interval of 1 minute is really required for calender events.

• adapt testing to supported node releases

  Consider adding node 20 tests unless you already know incompatibilities.

  So the recommended testmatrix is [16.x, 18.x, 20.x],

Thanks for reading and evaluating this suggestions.
McM1957

Please add a comment when you have reviewed and fixed the suggestionsor at least commented the suggestions and you think the adapter is ready for a re-review!

Additional info for @Apollon77
Please evaluate object / state usage. I do not know suffiently dataiuls for objects of type 'group' (https://github.com/dirkhe/ioBroker.webcal/blob/1abacf1afe0e1f068228b3c1995fdbfddb550c85/io-package.json#L207)

[dirkhe]

Hi @mcm1957 Thank you very much for your detailed feedback.

errors reported by adapter checker must be fixed

I add additional housekeeping for setInterval and setTimeout

reevaluate state roles

if I read the doc again, it is is ok, to set read to false. But for me it is not always clear, what have to use when. And if I check a lot of adapters, it will be handled differently.. But it is ok, With other states I tried to read the doc and find out the correct one. For the "fetchCal" button I thought, it could be useful to see, that button is pressed an released. But it is ok, no matter to discuss about

check and limit intervals used to access cloud services

You are right here. I set a min-flag in the JSON for 10 minutes, so user input should be validated, but I didn't check this in the code afterward. I added a check and set to 10 minutes if value is lower. btw. Default is 60 minutes.

adapt testing to supported node releases

is in place now

objects of type 'group'

I used this, because this is the entry point of different events, created by userinput in config. An also, perhaps better solution, could be "folder" here.

[github-actions]

Automated adapter checker

ioBroker.webcal

👍 No errors found

• 👀 [W202] Version of package.json (1.0.6) doesn't match latest version on NPM (1.0.5)
• 👀 [W400] Cannot find "webcal" in latest repository

Add comment "RE-CHECK!" to start check anew

[mcm1957]

ad buttons / switches

Yes, many adapters use it in different and often in a non planned way.
The general rule is:

BUTTON
A button has no value (in theory). Its value will always be true (at least after first press) and the adapter code should only trigger at a write (val=true, ack=false). Nobody might expect that the value will ever go to false again. The current (or near-future) admin version enhances this as the button will not show any colour change in future.

SWITCH
If you want to see 2 states, a switch should be used. A switch can be set to true and false - eitehr externally oder by adapter code if this is applicable. VIS will show the two states for switches.

Hope this makes it more clear.

[mcm1957]

Thanks, lgtm now

@Apollon77
Please evaluate object / state usage. I do not know suffiently dataiuls for objects of type 'group'
https://github.com/dirkhe/ioBroker.webcal/blob/6beaefc47dbf45bc0f53e950b7a9f85f5f28b407/io-package.json#L208

[dirkhe]

Thanks, lgtm now

@Apollon77 Please evaluate object / state usage. I do not know suffiently dataiuls for objects of type 'group' https://github.com/dirkhe/ioBroker.webcal/blob/6beaefc47dbf45bc0f53e950b7a9f85f5f28b407/io-package.json#L208

I changed type to "folder" instead of "group". If I see the icon in admin, it seems more like user-groups

[mcm1957]

@Apollon77 - ping, this adapter is waiting for a review by you

[mcm1957]

@Apollon77 - ping, this adapter is waiting for a review by you

[mcm1957]

@Apollon77 - ping, this adapter is waiting for a review by you

[dirkhe]

RE-CHECK!

[mcm1957]

RE-CHECK!

[mcm1957]

As Apollon77 is still short of free time, I'll release this adapter now.
Discussion at forum (https://forum.iobroker.net/topic/62479/test-neuer-adapter-webcal/186?_=1702750938522) proves that it is working sufficiently.