Add TeslaFi to latest

[hombach]

No description provided.

[simatec]

jsonConfig looks good...

[hombach]

"xs" should specify a value of "12" at admin/jsonConfig.json/items/_StandardTab/_donate -> has to be 6, otherwise button is too big

I'm no friend of automated translations - so I don't use i18n

[mcm1957]

Your TOO fast :-)
Size check has been disabled for divider an staticImage in the meantime. Checks are adjustes to results in real life. :-)

I18n is a warning only which will not be considered blocking.

[hombach]

:) - so all this 5 times 12 for dividers could be removed?.... -> next release ;)

... so must be fixed label isn't relevant anymore? Or do I have to change something?

[mcm1957]

Sorry, but due to travel / absence I will not able to do the review before ioBroker Meeting Solingen next weekend.

reminder 4.11.2024

[mcm1957]

LOL - you're right - I'm stupid ;)

thats defintstly wrong !

[mcm1957]

REVIEW - WORK IN PROGRESS

• TeslaFi related to tibber?

  Is teslafi related to tibber or is this loga a copy/paste error?

  'pull of homes from Tibber-Server'

• unused onMessage handler

  onMessage handler has no obvious functionality. Please remove from code an config.

• 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, '_');
}

• avoid usage of setObject

  setObject(Async) creates a new object whenever it is called. You should avoid using setObject unless you really want to create new objects. Creating a new object deletes any userConfig, i.e. configures history logging which is most likely not desired. Most likely either setObjectNotExists oder (even better in most cases) extendObject should be used. Extend Object ensures that attributes like role, read/write flag etc. are set as specified and hence extendObjet should be preferred when called once during startup of the adapter.

• 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 functionality 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.

  One example: role 'value' must not be writeable.

• Is there any reason to not use i18n translation environment?

  I suggest to evaluate to use i18n translation environment and the iobroker/adapter-dev environment (https://github.com/ioBroker/adapter-dev#iobroker-adapter-dev). You could easyly add new strings to the english json and run npm run translate afterward. When using i18n environment activating iobrokers weblate translation system (see https://github.com/ioBrokerTranslator/doc) is trivial.

• Add timeout to axios calls

  All axios calls should have a timeout set. The default value for axios timeout is 0 which results in no timeout. So an unresponsive remote system could lead to an hanging adapter. Either set an appropiate timeout per axios call or set a default timeout globally for the axios instance.

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

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

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

• linter setup

  Please use standard iobroker linter setup. This is located at @iobroker/eslint-config. See migration guide at https://github.com/ioBroker/ioBroker.eslint-config/blob/main/MIGRATION.md

[mcm1957]

@hombach
Thanks for fixing most issues, The following issues still need attention:

• avoid usage of setObject

setObject(Async) creates a new object whenever it is called. You should avoid using setObject unless you really want to create new objects. Creating a new object deletes any userConfig, i.e. configures history logging which is most likely not desired. Most likely either setObjectNotExists oder (even better in most cases) extendObject should be used. Extend Object ensures that attributes like role, read/write flag etc. are set as specified and hence extendObjet should be preferred when called once during startup of the adapter.

You are using setObject i.e. here:
https://github.com/hombach/ioBroker.teslafi/blob/60b533418f207cdfa9fd88a7d8c46ce3b2e82aa8/src/lib/projectUtils.ts#L179

Wouldn't it be sufficient to use extendObject? setObject will delete the old object and hence delete all user defined attribute, i.e. setting for history adapter.

• 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 functionality 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.

role 'value' must not be writeable.
https://github.com/hombach/ioBroker.teslafi/blob/60b533418f207cdfa9fd88a7d8c46ce3b2e82aa8/src/lib/projectUtils.ts#L218

role 'indicator' must not be writeable.
https://github.com/hombach/ioBroker.teslafi/blob/60b533418f207cdfa9fd88a7d8c46ce3b2e82aa8/src/lib/projectUtils.ts#L269

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

You're using setInterval here
https://github.com/hombach/ioBroker.teslafi/blob/60b533418f207cdfa9fd88a7d8c46ce3b2e82aa8/src/main.ts#L71

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 add a suitable lower limit too:
  https://github.com/hombach/ioBroker.teslafi/blob/60b533418f207cdfa9fd88a7d8c46ce3b2e82aa8/src/main.ts#L71

reminder 16.12.2024

[hombach]

-> avoid usage of setObject

This is only used for adapter setup.... you have to read the total code - automatic scanning for key words dosn't work this time....
if (!forceMode) { await this.adapter.setObjectNotExistsAsync(stateName, { type: "state", common: commonObj, native: {}, }); } else { await this.adapter.setObjectAsync(stateName, { type: "state", common: commonObj, native: {}, }); }

projectUtils.ts is a helper class I'm using in most adapters

-> reevaluate state roles

same, projectUtils.ts is a helper class I'm using in most adapters. The fact it's able to set up writable states doesn't tell it's used in the adapter. IUpo to now, all states are read only, as they are just states polled from the car. They have special background for the car but most of them no dedicated type in ioBroker. So I would prefer generall roles.

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

changed in 0.4.3

-> Limit all intervals used to access cloud services

allready done .... jsonConfiig.json:
"UpdateInterval": { "type": "number", "label": "LBL_UpdateInterval", "hidden": "!data.TeslaFiAPIToken", "min": 10, "max": 86400, "step": 1,

added extra verification in code

[hombach]

DONE

[mcm1957]

-> avoid usage of setObject

Well extendObject would be the better solution anyway in my oppinion. But finally it's your decision as using setObject is strongly discouraged but not forbidden. So its non blocking.

-> reevaluate state roles
The mentioned roles must not be used with writeable statesat any time. So there is no need to allow the code to set writeable to true. I do not see why code creating incorrect states should be present. Please remove writeable option or pass role as parameter setting it correctly. As you use this code at several apaters the fix shpould be applied everywhere to avoid incorrect states.

[hombach]

Hi Martin, adding the role as optional parameter in case of setting up writeable states is a good idea... implemented in 0.4.4.

[hombach]

added a few more precise state roles in 0.4.5

[mcm1957]

@hombach
This adapter has been released to latest repository and should be visible within 24h maximum.

Please create a thread at https://forum.iobroker.net/category/91/tester titled like "Test Adapter " to collect some user feedback and provide a link to this topic when requesting addition to stable repository later.

Note: If an other testing topic already exists, it' OK to continue using this topic too.

[hombach]

Hi Martin, kann gerne was posten - darf man aber ja in der Catagory nicht.

[mcm1957]

Als dev darfst du es.
Bitte ping HOMORAN via PN / Chat an dass er dich freischaltet