Synology-Chat adapter

[phoeluga]

No description provided.

[GermanBluefox]

Automated adapter checker

ioBroker.synochat

👍 No errors found

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

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

[phoeluga]

https://forum.iobroker.net/topic/47210/synology-chat/143

[Apollon77]

Hi,

here are some review comments:

• Must: Please directly state the License in https://github.com/phoeluga/ioBroker.synochat#license ...at least "it is MIT" or such ...
• Must: Please adjust this PR to sort in the adapter entry alphabetically :-)
• Info: This https://github.com/phoeluga/ioBroker.synochat/blob/master/main.js#L44-L47 should never happen because the default values shlould always be there.
• Info: This https://github.com/phoeluga/ioBroker.synochat/blob/master/main.js#L85 will currently break in Nodejs 18 ... maybe gets reverted in Nodejs (Revert the change of network interfaces family from String to Integer nodejs/node#43014) but maybe check also for family === 4
• Hint: Maybe ,move https://github.com/phoeluga/ioBroker.synochat/blob/master/main.js#L205 some lines down after the first connectivity check?

The rest is fine. Please adjust the two topics above and poke here agin. Then we are ready to merge

[Apollon77]

PS: YOu also misses axios in your depenednecies in package.json! Anything you need and require during runtime needs to be in dependencies and not devDeps!!

[phoeluga]

Hi,

Thanks for reviewing.
I adjusted the topics you mentioned accordingly.

1. Hint of MIT license was added directly in the Readme
2. Sorted the new synochat adapter alphabetically in the adapter list
3. Thanks for the hint, I'll keep that in mind for an upcoming release
4. An exception handling was added to keep a default value in case of using Nodejs 18
5. The subscription was added after the initial connectivity check
6. Added the missing dependencies

Please have a look again.

Thanks,
phoeluga

[Apollon77]

Hi,

thank you:

An exception handling was added to keep a default value in case of using Nodejs 18

The try catch will not help because it will never be an exception. the check with only be always false . maybe better to adfjust like

if ((details.family === 'IPv4' || details.family === 4)&& details.internal === false)

then you cover both.

Please release a new version to npm with the adjustments and we can merge

[phoeluga]

Hi,

Thanks for the hint, got your point now.
I removed the try catch again and adjusted the check:
https://github.com/phoeluga/ioBroker.synochat/blob/master/main.js#L89

A new version was published.

[Apollon77]

Thank you and welcome to the repository