Add air-q adapter to latest

[pr0crstntr]

Please consider adding our air-q adapter to the latest repo.
The thread for the testing forum is here: https://forum.iobroker.net/topic/70188/test-adapter-air-q-air-q-luftanalysator-v0-0-x/4

Thank you in advance!

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

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

If you have any urgent questions feel free to ask.

mcm1957

[mcm1957]

reminder 27.02.2024

[mcm1957]

just a short feedback before offical / detailled review:

-) please fix README, installations instructions for dev-server are completly offtopic for users and installation ov an adapter should never ever be sugggested / performed using npm. (Installation shopuld be performed by admin UI or iobroker commandline)

-) fix issues reported by adapter checker (errors only)

-) check and fix standard github based testing. Tests must pass.

[mcm1957]

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

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.

• errors reported by adapter checker must be fixed

❗ [E118] Versions in package.json and in io-package.json are different
❗ [E606] Current adapter version 1.0.0 not found in README.md
❗ [E701] No actual year found in LICENSE. Please add "Copyright (c) 2024 Katharina K. [email protected]" at the start of LICENSE

• Readme.md need rewrite / adaption

  Please remove installtion section completly. Users should install an adapter using standard ioBroker mechanism (admin interface or iobroker commandline).

  Installing dev-server is a development task and must not be done by a normal user.

  Never ever a n ioBroker adapter must be installed using npm. This is an absolute no-go as this will miss several stepos (i.e. admin upload) and is likely to destroy the complete ioBroker installation is done insoede the wrong director or using the wrong user.

  So please remove installation instructiony before the pictured "you should be able to find the adapter through the interface" area. The further steps seems to be clear and thanks a lot for describing the configuration in detail.

  Please remove section testing wiuth dev-server. Thats a developer task only and must not be done by users
  https://github.com/CorantGmbH/ioBroker.air-q?tab=readme-ov-file#test-the-adapter-manually-with-dev-server-

• Readme.md does not contain changelog for latest release

  The changelog at README.md of this adapter does not contain an entry for the current release available at npm. Please update README.md. (A new release is not required, an update at github is sufficient.)
  https://github.com/CorantGmbH/ioBroker.air-q?tab=readme-ov-file#changelog-

• please verify that all dependencies are up to date

  Please check whther eslint-plugin is really needed by users at production system. This is unusual. Normally this sould be a dev-dependency only
  https://github.com/CorantGmbH/ioBroker.air-q/blob/d51222cfaf4c34f8ecdcc2b1cd95627d431969a3/package.json#L29

• sensitive information, especially passwords should be stored encrypted

  You configuration seems to contain passwords (or other sensitive information) which is not stored encrypted. As long as those information is not stored inside a table you can encrypt and secure this infomation simply by adding the following configuration to io-package.json. The iob runtime will encrypt / decrypt the data as required; no addition programming effort is required. Note that users will need to reenter this info one time after adding encryption, so drop a note at release notes.

  "encryptedNative": [
    "password",
    "data2"
  ],
  "protectedNative": [
    "password",
    "data2"
  ],
  

• consider enhancing error messages

  If axios throws an error only a fixed error text ('Error while getting sensors from device) is displayed. This will make it more complicated to analyze user problems. I suggest to output more information provided by axios (i.e. https codes etc and / or the complete error text) so that you can check whats the reson for an error. User may have network troubles (firewall, dns, ...) or timeouts or other problems. You will not be able to help the user if theres only an fixed error text.

  https://github.com/CorantGmbH/ioBroker.air-q/blob/d51222cfaf4c34f8ecdcc2b1cd95627d431969a3/src/main.ts#L255

• consider using jsonConfig

  You configuration seesm to be ruther simple. So you should consider using jsonConfig instead of meterialize for new adapters. Materialize is still OK but sowhow deporecated. New adapters should use jsonConfig or react id jsonConfig is not sufficient.

  But consider this as a suggestion. This issue is NOT blocking

• use std info.connection state

  Please use standard info.connection state instead of private state to signal connection to the device. See adapter template or other adapters for details. info.connection will be handled by admin to show state of adapter.

  https://github.com/CorantGmbH/ioBroker.air-q/blob/d51222cfaf4c34f8ecdcc2b1cd95627d431969a3/src/main.ts#L36

• invalid characters must 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 create states based on information read from external device. Those IDs must be filtered to remove potentially invalid characters. See i.e. https://github.com/CorantGmbH/ioBroker.air-q/blob/d51222cfaf4c34f8ecdcc2b1cd95627d431969a3/src/main.ts#L91

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

  I cannot detect any useful action inside you onStateChange code. The code only reads a state but does not wait for the result nor does it try to use the result.
  https://github.com/CorantGmbH/ioBroker.air-q/blob/d51222cfaf4c34f8ecdcc2b1cd95627d431969a3/src/main.ts#L275

  In addition you call the onStateChange routine manually for whatever reason. Please explain what you are trying to do here. the onStateChange handler will be called whenerver a subscribes state changes. I cannot see any reason to call it this way. I think this code should be removed completly.

• reevaluate subscribeStates

  You subscribe to states. But I do not see a code which reacts onto any value written to the states subscribed. Are you sure that those states are ment to be written by users / are to be used as INPUT to the adapter?

• reevaluate write flags for states

  Only states which are used as input to the adapter and hence are to be written ba users (i.e. zu set a new temperatur level) must have attribute write:true. States which are only written be ythe adapter (i.e. measuremnt values provided) must have write:false set.

• 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) whnever a dedicated role exists.

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

• axios calls must have a timeout specified

  The default timeout for axios is 0 meaning no timeout. If a remote system is not responding, this could lead to a hanging adapter. So please add a timeout to all axios calls (or set it as default for all calls) to ensure that axios aborts calls after a defined and acceptable time.

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

  Please ensure that retrievalRate is set to a value larger than axios timeout. As an alternative replae setInterval by setTimeout to restart the next loop once the previous is completed. Multiple parallel running processing loops must be prohibited as this would sum up and eventually crash the ioBroker system due to overload (i.e. excessive memory consumption)

• check and adapt testing

  Standard iobroker testing is present but fails. Please fix

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]

reminder 14.3.2024

[pr0crstntr]

Thank you so much for your patience and the helpful review! It seems like i have misunderstood the onStateChange handler and have removed it from the code. I have pushed the new changes to our main branch and used the adapter checker for more errors. If there is anything else you'd like me to change or see an issue with, let me know!

[mcm1957]

Thanks for adapting the code. I'll do a review as soon as I have time.
Anyway if you have an questions (i.e. on the pupose of onStateChange handler oder other interfaces) please feel free to jpoin aur telegram developer channels (especially the starters one) and ask whatever you want to know to create an adapter. Invite Links are available at www.iobroker.dev. (non starters channel is open for all devs too)

[mcm1957]

Sorry for the dealy for the review. I've one point still marked as open:

• use std info.connection state

  Please use standard info.connection state instead of private state to signal connection to the device. See adapter template or other adapters for details. info.connection will be handled by admin to show state of adapter so it should be used whenever an adapter has some sort of connection to a device

  https://github.com/CorantGmbH/ioBroker.air-q/blob/d51222cfaf4c34f8ecdcc2b1cd95627d431969a3/src/main.ts#L36

  Is there any reason to not use the standard way to signal connection to device? You can simply define the objet at io-package.json (see https://github.com/iobroker-community-adapters/ioBroker.envertech-pv/blob/97fc6cc66055bed259b528cc500245b623df6c84/io-package.json#L179) and access it within your code

  Please adapt or comment if there is any reason to use our slightly other implementation.

reminder 15.3.2024

[pr0crstntr]

Right, there was no specific reason for it to be private, I added some more changes to the adapter now. I appreciate the feedback and extra resources.

[github-actions]

Automated adapter checker

ioBroker.air-q

-

👍 No errors found

• 👀 [W171] "common.title" is deprecated in io-package.json
• 👀 [W156] Adapter should support admin 5 UI (jsonConfig) if you do not use a React based UI
• 👀 [W400] Cannot find "air-q" in latest repository

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

[mcm1957]

looks good now.
One last request: Please create a new release (1.0.1) so that the last fixes are included into npm package.

Please add a comment when you done. THANKS

reminder 12.3.2024

[mcm1957]

looks like new release has been created.
recheck it.

reminder 8.3.2024

[mcm1957]

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.