Add echarge_cpu2 to latest

[LembkeM]

No description provided.

[LembkeM]

The close was a mistake.

[mcm1957]

Link to adapter repo: https://github.com/LembkeM/ioBroker.echarge_cpu2

[mcm1957]

Result from manual adapter checker run:

[E201] Bluefox was not found in the collaborators on NPM!. Please execute in adapter directory: "npm owner add bluefox iobroker.echarge_cpu2"
[W400] Cannot find "echarge_cpu2" in latest repository

[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.

• errors reported by adapter checker must be fixed

  Especially error [E201] Bluefox was not found in the collaborators on NPM!. must be fixed. If you just invited Bluefox its ignore to wait for his reaction. Please state that the invitation has been sent in this case.

• Readme.md has not been created

  The README.md of this adapter is still nearly ident to the template created by adapter creator. Please add a short description about the functionality of the adpter, add links the the manufacturer site (if applicable) and add a description of the configuration of the adapter. If you provide an external documentation - which should be located at doc/xx directory - please add links to at least the english documentation.

  see https://github.com/ioBroker/ioBroker.repositories#requirements-for-adapter-to-get-added-to-the-latest-repository especially point publish stable version from landroid #4.

• jsonConfig.json seems to contain template data

                "checkbox1": {
                    "type": "checkbox",
                    "label": "Simple Checkbox",
                    "tooltip":"click to enable or disable checkbox, check tooltiüp by hovering over checkbox",
                    "help":"Helptext possible",
                    "default":false,
                    "newLine": true

• admin config seesm to contain react AND jsonConfig

  admin subtree seems to contain some react code but also jsonConfig.json. OIs this really desired or are there some outdated files still present?

• translation is missing

  i18n files are empty

• password chcking code

  What purpose has this code part ?

		// examples for the checkPassword/checkGroup functions
		let result = await this.checkPasswordAsync("admin", "iobroker");
		this.log.info("check user admin pw iobroker: " + result);

		result = await this.checkGroupAsync("admin", "admin");
		this.log.info("check group user admin group admin: " + result);

further checks aborted

Some more quick check notes only:

The main code seems to read information from the device exactly ONCE. I cannot assume that this is the desired behavior. If any exception is raised nothing is logged at all (catch area is empty)

onStateChange code exists but is not used

Several paramaters seem to be unused at code, passwords should be encrypted

  "native": {
    "basicDeviceUrl": "",
    "serverPort": 443,
    "userName": "Installer",
    "userPassword": "",
    "createRoomsAndFunctions": false,
    "webInstance": ""
  },

roles should be checked. See info.* roles for matching roles to avoid the general text role if possible.

I suggest to close this PR as this adapter seems to be work-in-progress currently. Please create a new PR as soon as the adapter is complete and has all functionality implemnted desired for the first release. Adapters added to latest repository must be already working.

Thanks for reading and evalutaing this suggestions.
McM1957

[LembkeM]

These steps are already done.
I'm waiting on Bluefox.

[mcm1957]

OK, adapterchecker is OK now, llogo is OK and Releasenotes are a little bit short - would be fine to read what functionality the adapter provides - but this will be definitly no blocking point.

BUT
I still cannot see the functionality of the adapter. As far as I see, the code reads some configuration data exactly one time. onReady is executed exactly ONCE whenever the adpter is started. Its not called periodically at all.

An the other remarks (like template code without any functionality recogniozed) is still present.

So please could you describe a little bit more in detail, what you think the adapter is doing and check the unused onState code and explyin the password checking routines expected functionality.

Maybe I'm incorrect - but the code looks like a proof of concept currently with not much real functionaliy. If I missed soimething - I'm sorry - but please respond to the sugestions:

• jsonConfig.json seems to contain template data

                "checkbox1": {
                    "type": "checkbox",
                    "label": "Simple Checkbox",
                    "tooltip":"click to enable or disable checkbox, check tooltiüp by hovering over checkbox",
                    "help":"Helptext possible",
                    "default":false,
                    "newLine": true

• admin config seems to contain react AND jsonConfig

  admin subtree seems to contain some react code but also jsonConfig.json. OIs this really desired or are there some outdated files still present?

• translation is missing

  i18n files are empty

• password checking code

  What purpose has this code part ?

		// examples for the checkPassword/checkGroup functions
		let result = await this.checkPasswordAsync("admin", "iobroker");
		this.log.info("check user admin pw iobroker: " + result);

		result = await this.checkGroupAsync("admin", "admin");
		this.log.info("check group user admin group admin: " + result);

• general functionality question

  The main code seems to read information from the device exactly ONCE. I cannot assume that this is the desired behavior. If any exception is raised nothing is logged at all (catch area is empty)

• onStateChange code exists but is not used

  If you do note react on state changes, there should not be any onStateChange code. This is unneeded overhead in this case.

• unused paramaters declared

  Several paramaters are specified at io-package.json but seem to be unused at code, passwords should be encrypted

  "native": {
    "basicDeviceUrl": "",
    "serverPort": 443,
    "userName": "Installer",
    "userPassword": "",
    "createRoomsAndFunctions": false,
    "webInstance": ""
  },

• usage of roles

  roles should be checked. See info.* roles for matching roles to avoid the general text role if possible.

• check and adapt testing

  Standard iobroker testing is required but seems to be missing. Please add and setup ./gizthub/workflows/test-and-release.yml

• adapt testing to supported node releases

  Tests for node 16 is mandatory unless you require node 18 and higher. (already OK)
  Tests for node 18 is mandatory as many users already use node 18 and node 16 will be eol and deprected Q3/2023. (already OK)
  Consider adding node 20 tests unless you already know incompatibilities.

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

[LembkeM]

• admin config seems to contain react AND jsonConfig

Hi, almost done.
Can you give me a hint, or a link for those two topics?

• admin config seems to contain react AND jsonConfig
• usage of roles
  That would help a lot.

Thanks

[mcm1957]

Roles see https://github.com/ioBroker/ioBroker.docs/blob/master/docs/en/dev/stateroles.md
There are some more for power / energy upcoming. See open pr at project.

JsonConfig /react
Please ask at telegram. I think most files except jsonConfig.json an i18n Tree can be removed ar admin/.... But there are better experts at telegram to help.

[mcm1957]

Discord woild be ok too as it is mirrored at telegram

[LembkeM]

Roles see https://github.com/ioBroker/ioBroker.docs/blob/master/docs/en/dev/stateroles.md There are some more for power / energy upcoming. See open pr at project.

JsonConfig /react Please ask at telegram. I think most files except jsonConfig.json an i18n Tree can be removed ar admin/.... But there are better experts at telegram to help.

Thanks for the link.
I've added some more suiteable roles.
So next one finished.

So last one needs to be finished.
Thanks

[LembkeM]

Hi,
is here somthing needed from my side?
Thanks

[mcm1957]

No - currently no action required from your side as far as I can see.

Sorry for the long delay. Apollon77 will take care of the (meantime large) number of open latest request after his holidays. Please be patient one or two additional weeks.

[LembkeM]

No - currently no action required from your side as far as I can see.

Sorry for the long delay. Apollon77 will take care of the (meantime large) number of open latest request after his holidays. Please be patient one or two additional weeks.

That's not a problem, thanks for the reply.

Holidays are important and hope he will have good ones.

Thanks for your help.

[github-actions]

Automated adapter checker

ioBroker.echarge_cpu2

👍 No errors found

• 👀 [W400] Cannot find "echarge_cpu2" in latest repository

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

[mcm1957]

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

• consider using adapter.setTimeout / this.setTimeout instead of (standard) setTimeout

  The adapter package provides wrapper routines for native setTimeout, setInterval, clearTimeout and clearInterval. Using those routines ensures that timers are cancelled on on load. Additional checks on thomse limits might be performed, too. So consider replacing native setTimeout/clearTimeout by adapter.setTimeout/adapter.clearTimeout or this.setTimeout/this.clearTimeout. The same refers to setInterval/clearInterval.

• timeouts and intervals must be cancelled

  At https://github.com/LembkeM/ioBroker.echarge_cpu2/blob/c47f299ffd83bbaf42d346d7984c2cf6d2d15823/src/saliahttpservice.ts#L59 you start an intervall time of 10s to refresh data. This intervall timer MUST be cancelled during onUnload. I did not dtect such a clear cancel - as you do not save the time id I think its not possible anyway.

• recurrent info logs

  It looks like ChargeData is refreshed every 10s and this event is logged. (https://github.com/LembkeM/ioBroker.echarge_cpu2/blob/c47f299ffd83bbaf42d346d7984c2cf6d2d15823/src/main.ts#L200)
  Consider changing this log to debug level as its not desired to fill the log with recurrend infos during operation which are not related to errors.

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!

[mcm1957]

How shall we continue?
Please let us know if you plan zu fix the noted issues in the near future - or as an alterntive - like to discuss some / all of those points.

[mcm1957]

@LembkeM

Any news?
Do you need help fixing the issues?

Please let us know if you still are woirking / maintain this adapter - a simple I'll continue as soon as I have time would be ok.

If there is no reaction until 15.12.2023 this PR will be closed.

@Apollon77 FYI

[mcm1957]

@LembkeM

A kind reminder.

Please drop any comment if you still maintain this adapter and will fix testing matrix in a resonable time.
If you need help, let us know.

If there is no reaction until 15.12.2023 this PR will be closed.

@Apollon77 FYI

[mcm1957]

As there has been no reaction to latest comments.I will close this PR now.

THANKS that you spent time to implement an adapter. If you find time to finalize and release the adapter at any tim ein future you are very welcome. Please simply create new PR in this case.

If you have any quetsions, feel free to contact us

@mcm1957
@Apollon77