Add Solaredge adapter to latest

[92lleo]

Add Solaredge Adapter
Tracked in 92lleo/ioBroker.solaredge#3

[Apollon77]

You also should:

• think about to upgrade dependencies
• add "request" package as dependecy if you use it (see testing failures)
• enter news in all languages (see translate tools in dev-best-practices in readme of this repo)
• add the default values of your two config vars to native section in io-package
• you set the values using default value of the created objects? this will not work and should never update the value on next run; also ideally use async/await then you also do not need that kill-timer. also https://github.com/92lleo/ioBroker.solaredge/blob/master/main.js#L146 will not work if the system is a bit slower ...
  Please check and fix

[Apollon77]

@92lleo Do you need any kind of support to adress the topics from above?

[92lleo]

@Apollon77 thanks for asking.
92lleo/ioBroker.solaredge#3 tracks your points.

• Do you have an example for async/await?
• Whats the problem with adapter.stop and slow systems? Is there a better way for scheduled adapters?

PS:
I knew that using createState to update the states is not the desired way, but it worked before the js-controller update, so I never bothered to change it.

[Apollon77]

• async/await: we just helped an other dev ... look added awattar adapter #986 (comment), else on discord and telegram are developer channels to also get infos and support
• no it is not about using adapter.stop, you need to, but you should make sure top call it when all logic is really done. all actions to create objects or set states are executed asynchronously. So if you assume that a system needs 5s to process all stuff then it is an asumption. your stop could kick in too early and so only parts of the data is set. thats why make sure that all stiuff is done and then directly call adapter.stop and you can be sure all is good

[92lleo]

Ah, thanks for the clarification. Changed async/timeout to async/await: 92lleo/ioBroker.solaredge@f1b1c64
Seems to work just fine. I'll see if I get it a bit cleaner, also the tests still need work, seems like I broke them when updating dependencies

[Apollon77]

cool, just poke when ready for next review

[Apollon77]

@92lleo Any update?

[Apollon77]

@92lleo Anything we can help with?

[ldittmar81]

Hello @92lleo are you ready for the next review? Do you need some help?

[Apollon77]

@92lleo How we should continue? Can we help somehow? Should we do a PR with relevant changes? But that would still require you to verify and test and release it afterwards because the changes can only be done "blind".

[92lleo]

Hi @Apollon77 , @ldittmar81
sorry to leave you waiting, thanks for your support! I haven't done much for that topic lately, as far as i can tell the biggest issue missing was the tests. If you can help out in any way it's highly appreciated. Tests&verficiation + release can be done from my side. I'll also check&update the correspoinding issue(s) over the next weeks

[Apollon77]

Please approve testing for 92lleo/ioBroker.solaredge#19

[92lleo]

@Apollon77 tested, reviewed & merged. Preparing the release this week. sorry for the long wait

[Apollon77]

@92lleo any news for the release? last step missing

[92lleo]

@Apollon77 sorry again for the long wait.. Release is pushed and tagged -> https://github.com/92lleo/ioBroker.solaredge/releases/tag/0.3.0

[mcm1957]

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

I would like to give some (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.

• scheduled adapters should have random delays

  An unmodified schedule adapter that queries data from external systems will produce a lot of issues because usually the default schedule stays unchanged for all users and so all installations out there will then contact the server at the exact same time create a "potentially huge" request spike. The best practice for now is at least to add a random delay to spread the calls within that minute - or even move the "minutes" on first start and so persist that (e.g. see https://github.com/ioBroker/ioBroker.yr/blob/master/main.js#L84-L90) ... if the adapter gets successfull then we have a big issue and the webpage owner contacting us ... we should prevent that from the beginning.

  io-package.json specifies "schedule": "*/15 * * * *" which will execute at minute 0, 15, 30 and 45 of every hour as far ass I know. Please ignore, if I'm wrong.

• github dependabot support seems to be missing

  It looks like you did not yet setup dependabot support; at least .github/dependaboit.yml is missing.

  Please consider setting up dependabot support (and to ease life the workflow auto-merge.yml). Dependabot will ensure that your adapter uses the most recent versions of external packages and avoids problem due to outdted base libraries. Dependabit provides PRs at you repository to update the dependencies. Depending on you setting, especially on auto-merge.yml configuration no or only minor revisions will be outmatically merged into you main branch. All major revisions must (ans should) be reviewed by you. But its fully OK if you wish to check dependencies manually too.

Thanks for reading and evaluating this suggestions.
McM1957

[mcm1957]

RE-CHECK!

[Apollon77]

Both your comments from @mcm1957 are correct and @92lleo should consider it, especially the random delays. Depending on how users use the data the delay should be static per instance that it is always 15 mins difference or it could be really dynamic (determined per run) ... @92lleo wdyt? And yes I could provide another PR for that if you like.

[mcm1957]

@92lleo

Please let us know if you are still maintaining this adapter and are intrested in registratrion at latest repositories.
Currently tests at adapter repo seem to fail:

If you are still interested I will do a new review to check what issues are still open / already fixed and to get this PR merged.

Please simüply post any reply (and check and fix github tests at repository).

If we do not get any responce with 30 days we will close this PR.
But of course a new PR will be welcome at any time.

[github-actions]

Automated adapter checker

ioBroker.solaredge

• ❗ [E118] Versions in package.json and in io-package.json are different
• ❗ [E605] No actual year found in copyright. Please add "Copyright (c) 2019-2023 92lleo [email protected]" at the end of README.md
• ❗ [E701] No actual year found in LICENSE. Please add "Copyright (c) 2019-2023 92lleo [email protected]" at the start of LICENSE
• 👀 [W171] "common.title" is deprecated in io-package.json
• 👀 [W105] "common.titleLang" should be translated into all supported languages (en, de, ru, pt, nl, fr, it, es, pl, uk, zh-cn)
• 👀 [W109] "common.desc" should be translated into all supported languages (en, de, ru, pt, nl, fr, it, es, pl, uk, zh-cn)
• 👀 [W156] Adapter should support admin 5 UI (jsonConfig) if you do not use a React based UI
• 👀 [W145] Each "common.news" should be translated into all supported languages (en, de, ru, pt, nl, fr, it, es, pl, uk, zh-cn)
• 👀 [W145] Each "common.news" should be translated into all supported languages (en, de, ru, pt, nl, fr, it, es, pl, uk, zh-cn)
• 👀 [W145] Each "common.news" should be translated into all supported languages (en, de, ru, pt, nl, fr, it, es, pl, uk, zh-cn)
• 👀 [W145] Each "common.news" should be translated into all supported languages (en, de, ru, pt, nl, fr, it, es, pl, uk, zh-cn)
• 👀 [W145] Each "common.news" should be translated into all supported languages (en, de, ru, pt, nl, fr, it, es, pl, uk, zh-cn)
• 👀 [W202] Version of package.json (0.3.0) doesn't match latest version on NPM (0.2.0)
• 👀 [W400] Cannot find "solaredge" in latest repository
• 👀 [W513] "gulpfile.js" found in repo! Think about migrating to @iobroker/adapter-dev package

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

[92lleo]

Hello @mcm1957
Unfortunately I don't have the capacity to do anything for this currently. If anyone is interested I'd be glad to move the repo to the comunnity repos. If it does not meet the requirements yet, lets just close the PR

[mcm1957]

Bypass commit by @GermanBluefox
9ff2c9e

[mcm1957]

PR will be checked / closed after transfer of adapter to community area

[mcm1957]

Adapter has been trasferred to iobroker-community-adapters orga.