Add fenecon to latest

[mcm1957]

Thanks for spending your time and providing a new adapter for ioBroker.

Your adapter will get a manual review as soon as possible. Please stand by - this might last one or two weeks. Feel free to continue your work and create new releases. You do NOT need to close or update this PR in case of new releases.

In the meantime please check any feedback issues logged by automatic adapter checker and try to fix them.

You will find the results of the review and eventually issues / suggestings as a comment to this PR. So please keep this PR watched.

If you have any urgent questions feel free to ask.

[mcm1957]

reminder 21.6.2024

[mcm1957]

@sg-app

First of all - THANK YOU for the time and effort you spend to maintain this adapter. And sorry for the long delay due to holiday absence and lack of time.

I would like to give some feedback based on my personal oppinion. @Apollon77 might have additional suggestions or even a different oppinion to one or the other statement. Please feel free to contact him if you cannot follow my suggestions or want to discuss some special aspects.

• [x] Readme.md is very short

  The README.md of this adapter is very minimalistic. 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.

• [x] Readme.md should contain a link to the manufacturer and/or device

  The README.md of any adapter related to device or m ultiple devices of a manufacturer should contain a link the manufacturers website and/or to a description of the device. This is to anable new users to easily check wether they associate the adapter with the correct device(s). (See https://github.com/ioBroker/ioBroker.repositories#requirements-for-adapter-to-get-added-to-the-latest-repository paragraph 4.)

• [x] package.json should contain useful keywords

  Please add some matching keywords to package.json. "template" is most likely no keyword related to this adapter. And I do not see a keywaord noting the adapterm the manufactorer or the functionality.

• [x] package.json should require up to date js-controller and admin version

  New adapters should request js-controller 5 and admin 6 as minimum if there are no strong reason to support (and test) older releases. So please add / adapt at io-package.json:

      "dependencies": [
          {
              "js-controller": ">=5.0.19"
          }
      ],
      "globalDependencies": [
          {
              "admin": ">=6.13.16"
          }
      ]

• [ ] invalid characters should be filtered from object ids

  Some characters are not allowed to be used as part of an object id. If an object id is not hardcoded or even depending on user input you should ensure that such an object id does not contain an invalid character. Iobroker provides the constant adapter.FORBIDDEN_CHARS, i.e. by using the following snippet:

  function name2id(pName) {
    return (pName || '').replace(adapter.FORBIDDEN_CHARS, '_');
  }

  You seem to receive the stateIds (or components of the stateId) from an external source. This infomation could contain invalid characters. So its required ths filter / replces the date using FORBIDDENCHARS constant. In addition dots and spaces must be removed. Spaces cause some vis apps to fail. Best practise for new adapters would be to allow 0-9,a-z,A-Z,,- only.

• [ ] folder objects must be created

  At https://github.com/sg-app/ioBroker.fenecon/blob/b87d32124a46070f7fc5f3eb138d71a0c160b6c2/main.js#L116 you create stateIds be concatenating strings with '.'. Dots within a stateId are used as folder (device, channle, state) seperator. You must create objects for every level. The code looks like you only create the state object (tree leeve) at the moment.

• [ ] consider caching existing states

  Currently a getState is executed whenever a state is updated. Ad this is only done to detect if the state must e created first I would suggest to cache the states already created an checked and to avoid the getState calls. Simply check if the stateId exists within an iobject. If not, use getState and create state if required. Afterwards store the stateId as key inside the object. This way getState is only called once per adapter start.

• [x] unused onStateChange handler

  You configred an onStateChange handler but it looks like this handler does not do any important tasks. Please remove the handler if the adapter does not react on state changes.

• [x] Why do you create any state with write:true access?

  As you do not process any write to states - you do not subscribe to any state ond you do not have a working onStateChange handler) wroiting to a state will have no effect. So please set write:false for all states.

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

  Please avoid using general roles (state, value) whenever a dedicated role exists.

• [x] 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.

• [x] check and limit configurable timeouts / intervals

  Node setTimeout/setInterval routines have a maximum allowed value of 2,147,483,647 ms. Using delays larger than 2,147,483,647 ms (about 24.8 days) result in the timeout being executed immediately. So all (user configurable) values passed to setTimeout / setInterval should be checked in code and limited. Checking/limiting in code is required as config data could be changed directly or by some other adapter too, so limiting at ui level might not be sufficient.

• [x] calculateAutarchiy code

  Does this code really work as expected? I doubt. When using getState / setState you should NOT add own instance as this part is added automatically bei get/setState. Only get/setForeignState requires a full stateId including namespace.

  Same error seems to exist at calculateSelfConsumption

• [x] adapt testing to supported node releases

  Tests for node 18 are mandatory. Tests for node 20 are mandatory as this is the recommended node version. Tests for node 22 should be added unless you already know incompatibilities which cannot be fixed immidiatly. In this case please create a issue stating the problem and fix as soon as possible.

  So the recommended testmatrix is [18.x, 20.x, 22.x] depending on engines requirments setting at package.json

  Tests must be performed at all supported platforms (linux, windows, mac) unless special hardware or software restrictions prohibit this.

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!

reminder 28.7.2024

[sg-app]

Hi, i have completed all points.

Why do you create any state with write:true access?

The feature for writing states was planned and will be implemented at a later date. I hope I was able to address all points to your satisfaction.

Georg

[mcm1957]

REVIEW - W.I.P

• [x] Readme.md is very short

  The README.md of this adapter is very minimalistic. 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.

• [x] Readme.md should contain a link to the manufacturer and/or device

  The README.md of any adapter related to device or m ultiple devices of a manufacturer should contain a link the manufacturers website and/or to a description of the device. This is to anable new users to easily check wether they associate the adapter with the correct device(s). (See https://github.com/ioBroker/ioBroker.repositories#requirements-for-adapter-to-get-added-to-the-latest-repository paragraph 4.)

• [x] package.json should contain useful keywords

  Please add some matching keywords to package.json. "template" is most likely no keyword related to this adapter. And I do not see a keywaord noting the adapterm the manufactorer or the functionality.

• [x] package.json should require up to date js-controller and admin version

  New adapters should request js-controller 5 and admin 6 as minimum if there are no strong reason to support (and test) older releases. So please add / adapt at io-package.json:

      "dependencies": [
          {
              "js-controller": ">=5.0.19"
          }
      ],
      "globalDependencies": [
          {
              "admin": ">=6.13.16"
          }
      ]

• [ ] invalid characters should be filtered from object ids

  Some characters are not allowed to be used as part of an object id. If an object id is not hardcoded or even depending on user input you should ensure that such an object id does not contain an invalid character. Iobroker provides the constant adapter.FORBIDDEN_CHARS, i.e. by using the following snippet:

  function name2id(pName) {
    return (pName || '').replace(adapter.FORBIDDEN_CHARS, '_');
  }

  You seem to receive the stateIds (or components of the stateId) from an external source. This infomation could contain invalid characters. So its required ths filter / replces the date using FORBIDDENCHARS constant. In addition dots and spaces must be removed. Spaces cause some vis apps to fail. Best practise for new adapters would be to allow 0-9,a-z,A-Z,,- only.

• [ ] folder objects must be created

  At https://github.com/sg-app/ioBroker.fenecon/blob/b87d32124a46070f7fc5f3eb138d71a0c160b6c2/main.js#L116 you create stateIds be concatenating strings with '.'. Dots within a stateId are used as folder (device, channle, state) seperator. You must create objects for every level. The code looks like you only create the state object (tree leeve) at the moment.

• [ ] consider caching existing states

  Currently a getState is executed whenever a state is updated. Ad this is only done to detect if the state must e created first I would suggest to cache the states already created an checked and to avoid the getState calls. Simply check if the stateId exists within an iobject. If not, use getState and create state if required. Afterwards store the stateId as key inside the object. This way getState is only called once per adapter start.

• [x] unused onStateChange handler

  You configred an onStateChange handler but it looks like this handler does not do any important tasks. Please remove the handler if the adapter does not react on state changes.

• [x] Why do you create any state with write:true access?

  As you do not process any write to states - you do not subscribe to any state ond you do not have a working onStateChange handler) wroiting to a state will have no effect. So please set write:false for all states.

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

  Please avoid using general roles (state, value) whenever a dedicated role exists.

• [x] 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.

• [x] check and limit configurable timeouts / intervals

  Node setTimeout/setInterval routines have a maximum allowed value of 2,147,483,647 ms. Using delays larger than 2,147,483,647 ms (about 24.8 days) result in the timeout being executed immediately. So all (user configurable) values passed to setTimeout / setInterval should be checked in code and limited. Checking/limiting in code is required as config data could be changed directly or by some other adapter too, so limiting at ui level might not be sufficient.

• [x] calculateAutarchiy code

  Does this code really work as expected? I doubt. When using getState / setState you should NOT add own instance as this part is added automatically bei get/setState. Only get/setForeignState requires a full stateId including namespace.

  Same error seems to exist at calculateSelfConsumption

• [x] adapt testing to supported node releases

  Tests for node 18 are mandatory. Tests for node 20 are mandatory as this is the recommended node version. Tests for node 22 should be added unless you already know incompatibilities which cannot be fixed immidiatly. In this case please create a issue stating the problem and fix as soon as possible.

  So the recommended testmatrix is [18.x, 20.x, 22.x] depending on engines requirments setting at package.json

  Tests must be performed at all supported platforms (linux, windows, mac) unless special hardware or software restrictions prohibit this.

[github-actions[bot]]

Automated adapter checker

ioBroker.fenecon

-

:thumbsup: No errors found

• [ ] :eyes: [W400] Cannot find "fenecon" in latest repository

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

[mcm1957]

Thanks, most remarks are solved. For those I have still remarks:

• [ ] invalid characters should be filtered from object ids

You filter illegal characters at object creation (https://github.com/sg-app/ioBroker.fenecon/blob/28e5fb459ac358c90b884acd4a5be6809e177ef1/main.js#L116).

But allowedId is only used at creation of object. When updating the state the unfiltered Id seems to be used. (https://github.com/sg-app/ioBroker.fenecon/blob/28e5fb459ac358c90b884acd4a5be6809e177ef1/main.js#L126)

This seems to be incorrect.

In addition I do not see a filtering for "channelName" https://github.com/sg-app/ioBroker.fenecon/blob/28e5fb459ac358c90b884acd4a5be6809e177ef1/main.js#L152

• [ ] folder objects must be created

You construct the state Id using address.jpin("."); https://github.com/sg-app/ioBroker.fenecon/blob/28e5fb459ac358c90b884acd4a5be6809e177ef1/main.js#L144 So your stateIds will look something like "aaa.bbb.ccc.ddd". So you MUST create each level seperatly, i.e. createObject ('aaa',...) --> type device or channel or folder createObject ('aaa.bbb',...) --> i.e. type folder (or channel) createObject ('aaa.bbb.ccc',...) --> type foleder createObject ('aaa.bbb.ccc.ddd',...) -> type state

If you are sure that there are always exactly two elements, you should check this at code to block incorrect data received. Do NOT trust that the webservice sends valid data only. (Often a webservice should send some structured rest data but in case of a problem a html page is transmitted. So you should check the data retrieved and block incorrect data from creating nonsense states etc.

• [ ] consider caching existing states You could (and should) avoid getObject calls only to detect if you already initialized them. As you call createUpdateState only during initalization

https://github.com/sg-app/ioBroker.fenecon/blob/28e5fb459ac358c90b884acd4a5be6809e177ef1/main.js#L126 https://github.com/sg-app/ioBroker.fenecon/blob/28e5fb459ac358c90b884acd4a5be6809e177ef1/main.js#L229 https://github.com/sg-app/ioBroker.fenecon/blob/28e5fb459ac358c90b884acd4a5be6809e177ef1/main.js#L255

you could remove the check wehtehr the object exists and execute extendObject always. extendObject creates the object if it does not exists and updates the object with the information passed to extendObject. If you do not want to update createObjectNotExists will do nothing if an object already exists.

So checks https://github.com/sg-app/ioBroker.fenecon/blob/28e5fb459ac358c90b884acd4a5be6809e177ef1/main.js#L147 https://github.com/sg-app/ioBroker.fenecon/blob/28e5fb459ac358c90b884acd4a5be6809e177ef1/main.js#L162 coudl be removed.

• [ ] reevaluate state roles

Please set correct state roles for all states. You might do this using some configuration table. Roles are important for vis and type detector. Setting all states to role "state" is a last chance way only and intended for adapter knowing nathing about the content they receive (i.e. mqtt). As this adapter handles values from an inverter, most or all states should be known and correct types and units should be set. You can add a mapping table for all known states and log new / unknown states so that the can be implemented at next release.

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!

reminder 2.8.2024