Add seplos-v3-sniffer to latest

[DpunktS]

Please add my adapter ioBroker.seplos-v3-sniffer to latest.

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

[github-actions]

ioBroker repository information about New at LATEST tagging

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. And please check the following information if not yet done:

• https://github.com/ioBroker/ioBroker.repositories?tab=readme-ov-file#requirements-for-adapter-to-get-added-to-the-latest-repository
• https://github.com/ioBroker/ioBroker.repositories?tab=readme-ov-file#development-and-coding-best-practices
• https://github.com/iobroker-community-adapters/responsive-design-initiative/tree/main#responsive-design-initiative

Important:

To verify the object structure of this adapter during REVIEW please export the object structure of a working installation and attach the file to this PR. You find a guide how to export the object struture here: https://github.com/ioBroker/ioBroker.repochecker/blob/master/OBJECTDUMP.md

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

If you have any urgent questions feel free to ask.
mcm1957

@simatec Please take a look in respect to responsive design. Thanks

[mcm1957]

reminder 20.2.2025

[DpunktS]

seplos-v3-sniffer.0.json

[mcm1957]

@DpunktS

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

• Please explain connection to ioBroker host better

  README.md says:

  Der Adapter erfasst die Kommunikation zwischen den Geräten passiv, wodurch die Kommunikation der einzelnen BMS nicht gestört wird. Er kann entweder über eine lokale Schnittstelle (z.B. ttyS0) oder über Ser2Net (tcp://ip:2001) kommunizieren.
  Es muss Pin 1/8, Pin 2/7 und Pin 5 mit eurem Adapter verbunden werden.

  In this context "adapter" seems to refer to some hardware not the ioBroker (software-)adapter. But I did not see any refernce which hardware adapter is required to use this ioBroker adapter.

• Is email [email protected] a valid mail address to contact you?

  Please use a valid email address which can be used to contact you. If you cannot be contacted using the email address specified at the configuration we might need to transfer and modify you adapter directly in case of problems. Of course the mail addresse need not (and should not be) your main address.

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

• objects 'bms' and '0'/'1' are not created at all.

  Those objects shoulde be created using device and channle types most likely.

• 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 check available roles and use more detailled roles when existing (i.e. for tempratures, voltage, current, ....)

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

• validation of input

  • tcp input should be validated. See https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/dbce1f568d0547e3bccf3c44a1f9ba50b39a477b/main.js#L65. Regular expression will do it already but user will get no clear feedback up the input is incorrect. Most likely only some tcp connect error will be shown.

  • format of data used as serialport is not checke in ay way too (https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/dbce1f568d0547e3bccf3c44a1f9ba50b39a477b/main.js#L96)

• avoid unneeded setObjectNotExists

  At https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/dbce1f568d0547e3bccf3c44a1f9ba50b39a477b/main.js#L267 you use setObjectNotExists for every state every time you received a value (typical fevery few seconds). This results in an avoidable system load. Please cache objectIds already used during a call to setObjectNotExists and avoid the call in those cases. This will result in calling setObjectNotExists only once after adapter restart while ensuring that new objects detected are created anyway. In addtion use of await setObjectNotExistsAsync should be prefered over using callback system (but the last remark is not mandatory).

• main.original, README.original

  Those files are obviously some "backup" file which does not fulfill any purpose. Please consider removing it. You can use github to see previous versions of any file anyway (thats the purpose of git :-) )
  (Is not considered blocking)

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!

Please add a new object dump after fixing creation of bms and 0/1/.. objects and reviewing roles.

reminder 27.2.2025

[DpunktS]

I have considered all suggestions and request a re-examination.
seplos-v3-sniffer.0.json

[github-actions]

Automated adapter checker

ioBroker.seplos-v3-sniffer

-

👍 No errors found

• 👀 [W401] Cannot find "seplos-v3-sniffer" in latest repository

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

[mcm1957]

WORK IN PROGRESS - This is a review checklist only - IGNORE FOR NOW

• Readme.md must 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.

• Please explain connection to ioBroker host better

  README.md says:

  Der Adapter erfasst die Kommunikation zwischen den Geräten passiv, wodurch die Kommunikation der einzelnen BMS nicht gestört wird. Er kann entweder über eine lokale Schnittstelle (z.B. ttyS0) oder über Ser2Net (tcp://ip:2001) kommunizieren.
  Es muss Pin 1/8, Pin 2/7 und Pin 5 mit eurem Adapter verbunden werden.

  In this context "adapter" seems to refer to some hardware not the ioBroker (software-)adapter. But I did not see any refernce which hardware adapter is required to use this ioBroker adapter.

• Is email [email protected] a valid mail address to contact you?

  Please use a valid email address which can be used to contact you. If you cannot be contacted using the email address specified at the configuration we might need to transfer and modify you adapter directly in case of problems. Of course the mail addresse need not (and should not be) your main address.

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

• objects 'bms' and '0'/'1' are not created at all.

  Those objects shoulde be created using device and channle types most likely.

• 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 check available roles and use more detailled roles when existing (i.e. for tempratures, voltage, current, ....)

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

• validation of input

  • tcp input should be validated. See https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/dbce1f568d0547e3bccf3c44a1f9ba50b39a477b/main.js#L65. Regular expression will do it already but user will get no clear feedback up the input is incorrect. Most likely only some tcp connect error will be shown.

  • format of data used as serialport is not checke in ay way too (https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/dbce1f568d0547e3bccf3c44a1f9ba50b39a477b/main.js#L96)

• avoid unneeded setObjectNotExists

  At https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/dbce1f568d0547e3bccf3c44a1f9ba50b39a477b/main.js#L267 you use setObjectNotExists for every state every time you received a value (typical fevery few seconds). This results in an avoidable system load. Please cache objectIds already used during a call to setObjectNotExists and avoid the call in those cases. This will result in calling setObjectNotExists only once after adapter restart while ensuring that new objects detected are created anyway. In addtion use of await setObjectNotExistsAsync should be prefered over using callback system (but the last remark is not mandatory).

• main.original, README.original

  Those files are obviously some "backup" file which does not fulfill any purpose. Please consider removing it. You can use github to see previous versions of any file anyway (thats the purpose of git :-) )
  (Is not considered blocking)

[mcm1957]

@DpunktS

Thanks for your work at this adapter an fixing most issues.
The follwoing remark need additional attention:

• avoid unneeded setObjectNotExists

  At https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/dbce1f568d0547e3bccf3c44a1f9ba50b39a477b/main.js#L267 you use setObjectNotExists for every state every time you received a value (typical fevery few seconds). This results in an avoidable system load. Please cache objectIds already used during a call to setObjectNotExists and avoid the call in those cases. This will result in calling setObjectNotExists only once after adapter restart while ensuring that new objects detected are created anyway. In addtion use of await setObjectNotExistsAsync should be prefered over using callback system (but the last remark is not mandatory).

  You added a cache of known objects as far as I see. BUT you anyway call ensureObjectExists for every update.

  Here https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/336501225472fcead3c8b2086645fc5a97ac23a3/main.js#L481 ensureObjectExists will be called whenever the update time has expired. So teh cached indicator is more or less without functionality.

  Here https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/336501225472fcead3c8b2086645fc5a97ac23a3/main.js#L234
  ensureObjectExists will be called for every packet received.

  I suggest that you move the check if the object exists into ensureObjectExists and siply do nothing if the oject exists.

  In addiotn a getOb ject is not required here https://github.com/DpunktS/ioBroker.seplos-v3-sniffer/blob/336501225472fcead3c8b2086645fc5a97ac23a3/main.js#L209 as setObjectNotExists does nothung if the object exists. So an extra call to check if the objet exists is not required.

  The goal is that setObjectNotExists is called only ONCE for every start of the Adapter. And calls to getObject should not be required at all for the purpose to checkif objects exist.

  Please review or comment if something has to be discussed.

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 7.3.2025

[DpunktS]

If I only call setObjectNotExists once at the start, it is not possible to react dynamically to the received messages. I would have to specify the number of batteries beforehand. I have designed the code so that the objects are created after data is received and are automatically created again after deletion. For example, if 2 batteries are connected, BMS0 and BMS1 are created. If the user then adds a battery, BMS2 is automatically created. Or do you have a better idea?

[mcm1957]

Thats completly ok to add data dynamically.

But you can and should do something like this:

const knownIds = [];
....
// check state, folder etc
if (!knownIds.includes(id) {
await setObjectNotExists(Id ....);
knownIds.push(id);
}
...

This will call setObjectNotExists only once after every Adapterstart. Checking the existence during processing of every packate this way is much mor cheaper than calling getObject. Of course you can use a hash / an object too instead of the array if you need to store some data anyway.

I hope this clearifies what I tray to say.

Of course this will not handle the case that a user deletes objects wile the adapter is running, But thats the case with 99% of all adapeters and nobody will or need to guarantee that such an action will work anyway.

P.S: Looks like this.knownObjects already is such a kind of cache. But it seems to miss some objects and seems to miss its functionality in conjunction with the timeout mechanism.

[DpunktS]

I hope I was able to fulfill all suggestions.
Please check again!