Add notificationforandroidtv to latest

[DNAngelX]

Please add my adapter ioBroker.notificationforandroidtv to latest.

This pull request was created by https://www.iobroker.dev c0726ff.

[mcm1957]

note from #3164

https://forum.iobroker.net/topic/71123/neuer-adapter-android-tv-fire-tv-benachrichtigungen/24?_=1704149932016

[mcm1957]

Why did you close this PR?

OK - seems to be an misundersatnding of web portal "add to latest". See remark at Forum.
https://forum.iobroker.net/topic/71123/neuer-adapter-android-tv-fire-tv-benachrichtigungen/54

[DNAngelX]

Why did you close this PR?

OK - seems to be an misundersatnding of web portal "add to latest". See remark at Forum. https://forum.iobroker.net/topic/71123/neuer-adapter-android-tv-fire-tv-benachrichtigungen/54

Yep, seem so, because there was an error while "add to latest" > there is a open PR,...

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

• Readme.md should be written in pure english

  As ioBroker is an international software supporting several languages the main Readme.md file should be written in english. Its ok to add additional Readme-.mds of course.

• Readme.md contains multiple languages

  Please remove the german version / all non english versions from Readme.md because Readme.md gets automatically translated and this will not work with multiple languages within Readme.md. You can add locatlized README-.md or add documentaion pages in (docs/de and docs/en) like https://github.com/iobroker-community-adapters/ioBroker.pvforecast/tree/main/docs and add links into the main README:md

• package.json should not package test scripts

  Test scripts (typically located at test directory are not needed at user systems. So they should not be included into npm package. Please remove form dir section

  Theres no need to add lib to directory section as files located at lib are already packaged by standard files section coontents.

  So complete dirs section should be removed from package.json.
  see https://github.com/DNAngelX/ioBroker.notificationforandroidtv/blob/b582fae8bdc64800b80f2e194cdd79b7ab617c85/package.json#L73

• please verify that all used dependencies are listed at package.json and are up to date

  At least axios is NOT listed as a production dependency. Please add axios and eventually other external packaged included by require to package.jsosn (best by using npm i command.

  Please verify that one package is not listed both at depencencies and dev-dependencies.

• consider using jsonConfig (non blocking)

  Please consider using jsonCOnfig as this adapter seems t o have very simple configuration interface. Metrialize UI should no longer be used for new adapters as long as jsonConfig is sufficient. Otherwise you should consider to use react.

  See message from checker: [W156] Adapter should support admin 5 UI (jsonConfig) if you do not use a React based UI

• native objects at io-package.json are incorrect

  io-package.json contains inital demo configuration objetcs (options1, options2) but is missing real configuration objects.
  https://github.com/DNAngelX/ioBroker.notificationforandroidtv/blob/b582fae8bdc64800b80f2e194cdd79b7ab617c85/io-package.json#L163

• Why reading state before writing?

  Its overhead to read every state before writing to it. If you really want to avoid writing unchanges values for whatever reason remeber the value at adapetr level (best practice) or use setStateChangedAsync

  see https://github.com/DNAngelX/ioBroker.notificationforandroidtv/blob/b582fae8bdc64800b80f2e194cdd79b7ab617c85/main.js#L80

• 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 be using the data entered by user at config without any filtering. The IP address could contain any other data i.e. as the user would enter ain IPv6 Address. Please add conversion or blocking of illegal characters. ote that dots are ONLY allowed to seperate between device / channel / folder and states.

• onStateChange handler ignores ack flag

  The adapter must honor the ack flag when processing onStateChange events. Whenever the onStateChange handler is called for an own state (a state owned by the processing instance of the adapter) it must ignore this event if the ack flags is set. Only stateChanges with ack==false are allowed to be processed.

  When processing foreign states which are output of another adapter processing must only occure if the ack flag is true.

• reevaluate state roles and write flag

  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.

  Please note that i.e. roles of type value must bnot be writeable, The same is true for indicator.

• axios seems to be missing a timeout

  The default for axios response timeout is ß0 indicating NO timeout. This can lead to a hanging adapter. Please add a timout value to all axios calls (or add it as a default)

• some labels seem to have no translation into all languages

  It seems that some labels are not translated into all supported languages, i.e. ...
  Please consider adding missing translations

• console.log should be replaced by this.log.xxx

  As console.log output is not visible at user system it seems to be better to use this.log.xxx with xxx i.e. error depending on severity of message

• minimize non exceptional logging

  Please review all log.info logging whther the really provide some benefit to users. Users often complain about adapter logging to much withgout real benefit. I would suggest to log the fact that that a message has been received only as log.debug. So it is available when looking for problems but does not spam the log file. (But this suggstion will not be considered blocking)

• check and adapt testing

  Standard iobroker testing is present but checks do not pass currently. Especially linter throws severyl errors. Many of them can be fixed using npm run lint --fix as suggested at log. Tests must show a green checkmark and run for all supported node versions.(This is already configured correctly)

  Use of var is outdated due to sideeffects caused by var. use let or const whichever is appropiate.

  Please avoid changing linter config or discuss before any changes performed. Lint tries to ensure a common programming standard for ioBroker adapters.

  Whether real tests are passing could be checked after linter passes.

  Please feel free to ask for support if needed.

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 28.01.2024

[DNAngelX]

• Readme.md should be written in pure english
• Readme.md contains multiple languages
• package.json should not package test scripts
• please verify that all used dependencies are listed at package.json and are up to date
• consider using jsonConfig (non blocking)
  I will try to do it in the next versions. There is not a really good description about that, I did not found anything related to "how add dynamicly lines" or like this. If I know where to search, no problem then.
• native objects at io-package.json are incorrect
• Why reading state before writing?
  first of all, this will be read only when the config is changed (how often you will buy and config a new android tv) or when the plugin boots up.
  The values I'm writing at the initial setup are suggestions from my site. IF a user will change it, the value from the user should be there and my value should never overwrite this again.
  I didn't saw a method which can handle this case to put values only at initial the device.
• invalid characters should be filtered from object ids
• onStateChange handler ignores ack flag
• reevaluate state roles and write flag
• axios seems to be missing a timeout
• some labels seem to have no translation into all languages

Yes, labels like type, icon URL and payload and all selections.
For selections I didn't found an working possibility how to do it.

const bkgcolor = {
	                0:"neutral blue",
	                1:"black",
	                2:"blue",
	                3:"green",
	                4:"red",
	                5:"light blue",
	                6:"turquoise",
	                7:"orange",
	                8:"purple"
	            }

• console.log should be replaced by this.log.xxx
• minimize non exceptional logging
  mostly debug now
• check and adapt testing

[mcm1957]

@DNAngelX

Standard checks are failing. Please fix. All adapters must use standadard github testing (as minimum) and tests must pass.

Errors reported by adapter checker must be fixed. You can run Repo Checker using www.iobroker.dev at any time too.
Native must be present at least as an empty attribute. Seems you removes lately.

[mcm1957]

reminder 12.2.2024

[DNAngelX]

@DNAngelX

How shall we continue? Do you need any help? Did you notice the last comment and the provided PR?

Please check and merge PR DNAngelX/ioBroker.notificationforandroidtv#14

As the PR might need an adaption in the meantime, please let me know if you need help.

Working / passing standard Github tests is mandatory for an adapter to be added to the repository.

If there is no reaction / comm ent until 30.4.2024 this PR will be closed

reminder 1.5.2024

Hey, I don't know what to do because the issues we faced up have nothing to do with the written code from me and relations.

It's something what came up after lint --fix, so I have absolute no idea what's the issue there and don't know how to fix that.

Everything from the request on Jan 14th was changed exact as expected.

So if after lint --fix some parts in the ground adapter are not working anymore,... I can't do there anything.

[github-actions]

Automated adapter checker

ioBroker.notificationforandroidtv

-

👍 No errors found

• 👀 [W171] "common.title" is deprecated in io-package.json. Please remove from io-package.json.
• 👀 [W156] Adapter should support admin 5 UI (jsonConfig) if you do not use a React based UI
• 👀 [W181] "common.license" in io-package.json is deprecated. Please define object "common.licenseInformation"
• 👀 [W173] "@iobroker/adapter-core" (^3.0.4) should be 3.0.6 or newer - please update
• 👀 [W115] common.tier is required in io-package.json. Please check https://github.com/ioBroker/ioBroker.docs/blob/master/docs/en/dev/objectsschema.md#adapter.
• 👀 [W028] Minimum node.js version 18 recommended. Please adapt "{'engines' : { 'node' >= '16' } }" at package.json.
• 👀 [W400] Cannot find "notificationforandroidtv" in latest repository

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

[mcm1957]

@DNAngelX

Hey, I don't know what to do because the issues we faced up have nothing to do with the written code from me and relations.
It's something what came up after lint --fix, so I have absolute no idea what's the issue there and don't know how to fix that.

Well github based tests are mandatory. Adapter code must be written so that they pass. So failing tests are related to the coe in some form. But we will try to fix this together - so no worry.

---

To get this PR done the following topics are open:

• Testing must pass

THANKS that you merged the older PR adding tests. As I stated some changes would be required in the meantime as supprot for node 16 has been dropped. (at testing). I have provides a PR to fix this for you.

• Tests failed as adapter did not do a clean start

I've analyzed your code in Detail. The reason for this problem is / was that you write the adapter using class style (that the current style and is ok) BUT in addition used adapter=util.adapter(....) which is the initalization to be used for old legacy, non class code. So you created to instzancey of teh adapter during startup causing problems later. I provided a PR to adapt your code to use class style (this.xxx) only.

So please check the PR DNAngelX/ioBroker.notificationforandroidtv#20 which fixes all issues causing problems with testing.

Please note that code review revealed two more suspect code places which should be checked by you. They are listed at the PR.

Please create a new release after merging the PR and check that the test are passing then. As soon as this is done, let me know by dropping a comment here.

reminder 1.5.2024

[mcm1957]

Testing seems to be fixed now. So no more blocking is present.
I did not see a co ment from your site - so I did not notice this until today.

[mcm1957]

@DNAngelX

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.

[DNAngelX]

Testing seems to be fixed now. So no more blocking is present. I did not see a co ment from your site - so I did not notice this until today.

Hey, thought the github-actions comment was enought :)

Better late then never I would say :)

THX