Roadmap to 1.0

[gfcittolin]

Hi @plcpeople and all users of nodeS7,

We all love nodeS7 the project and all the possibilities it opens. The code has been maturing along the last weeks and months, but I see some improvement room to codebase, and therefore I'd like to propose and discuss a roadmap to the version 1.0.0 of this awesome project.

In my personal opinion, in this first milestone I'd like to see (and of course contribute to) a major refactor of the code, separating the logic, creating classes, and organizing the code, without introducing any new feature. This would make it much easier to understand and to maintain, also making it easier to introduce many new features. We could then have more guarantees for the future of the project.

I think we could create a Wiki page or a Project to summarize all the ideas and discuss them, what do you think?

[Lichtjaeger]

Like it.

Here some of my points:

• separate code into multiple files (most necessary refactoring)
• make the connection object an event emitter ('connected', 'reconnected', 'disconnected', 'error', etc.)
• change the async actions (writeItems, readAllItems) from callback-based to Promise-based
• adding missing types and aliases #3

Some of my "nice to have":

• minimize the package size by adding an uglify build step
• make the package ESM ready

[plcpeople]

I agree the refactoring into multiple files/classes would be a huge first step.

I would also like there to be a much better way for the application to get the PLC connected status from nodeS7.

Should we create a Wiki page with some of these ideas? I think this is a good idea.

[gfcittolin]

I've been writing a sketch of it on the last few days, just posted on the Wiki: https://github.com/plcpeople/nodeS7/wiki/Roadmap-to-1.0

Feedback is always welcome :)

[chunzhu]

Hi everyone,

May i know how do you guys test if the functions did not break after some modifications? i would like to help contribute to the code base, but i do not have any experience in contributing on github, ensuring the codes are working as intended.

[gfcittolin]

As far as I know, there's currently no test specification to this library. I even just added this point to the Wiki document. Once we have the code better structured, we can test each part individually and how they integrate, making the contribution process a lot easier. Unfortunately, right now the contributor and/or the community need to test the changes manually before publishing something in order to prevent breaking something.

[chunzhu]

Noted. Thanks gfcittolin!

[ceremcem]

FYI: A helpful library for this kind of applications: https://github.com/ceremcem/signal (Since it's a relatively new spin-off library, it needs a little bit more documentation and a complete test suite, but it is actively used in our products so it may be considered stable)

[gfcittolin]

Hi guys,

I've been slowly writing some code related to our roadmap to 1.0, as much as my free time allows. In the last days I've published here on GitHub a sketch of an ISO-on-TCP implementation, as part of the stack needed to establish communication to a S7 PLC. I think this implementation could live as a separate packet/project, as there are other protocols on the wild that also use ISO-on-TCP as the transport stream and could benefit from it, such as RDP.

There's actually not much there yet, just a parser and a serializer implemented as transform streams. I've also written some tests to both based on captured packets of communications to S7 PLCs The idea would be to implement an ISO-on-TCP Socket class, extending the net.Socket class. The s7 connection could be built on top the this socket class, or it could only use the parser and serializer subclasses.

What do you guys think?

[babinc]

Maybe we should start a new branch? Could call the branch "Next" or "V1", or whatever. Just a suggestion. This would give us a safe place to work on v1.

[gfcittolin]

Indeed. If @plcpeople agrees, we could be set-up as collaborators, so we could directly work on a next branch. If not, we can work on one of our forks and then merge this back here later. I've been also thinking on putting some UML diagrams on the Wiki to better discuss with everyone how the code structure would look like.

[plcpeople]

OK, I can do that.

I had a look at the ISO-on-TCP code and it looks good. Would "fast acknowledge" eventually be part of this class?

[gfcittolin]

Thanks for the invite! I'm glad to contribute :)

Regarding Fast acknowledge, I don't have any device here nor any dump to simulate but, from what I understand, the "fast ack" (at least as it's currently implemented) is just a normal ISO-on-TCP DT telegram, but without data, and with the EOT bit unset, signalizing that that there's still more data to come. So this shouldn't be a problem for the class, just the implementation of nodes7 would have to handle these empty DTs accordingly (by discarding them, probably)

[sembaye]

Hi @gfcittolin great work and thanks for the initiative. Your thoughts about the FA telegram is correct. I already have some knowledge on this, which I am happy to share. I'm also able to test the FA!!!!!

I think it's best to start with a wiki page that we fill with life.

[chunzhu]

Hi All,

i have started refactoring some codes. May i know how should i go about contributing? do i just to a new pull requests? or create an issue ticket before a pull request?

Do guide me along, as i am very new to contributing codes here.

Thanks in advance!

[gfcittolin]

Hey, in my point of view the most logical way of refactoring it right now is writing each part/functionality as a separate piece of code (e.g. S7 protocol interpreter, S7 address translation and S7 endpoint), and then we can glue all together in the end. Sorry, I want to properly organize this structure, but I'm currently out abroad on a project until the end of the month, so it's kind of hard to find time for it. But as soon as I have at least a bit of it I'll post it here so we can discuss it together

[gfcittolin]

Hi everyone, Finally I had some time to sketch what I had in mind as a new structure to the project. You can find it here on the Wiki. It briefly describes there how to split the logic in multiple layers and files.

There's one main thing that I'd like to put on discussion: The library is currently focused on reading and writing based on a list of configured addresses, and we can (and hopefully will) implement many other features and functionalities, like reading random addresses (without being in a list first), drifting apart a bit from the read-all-items-centric profile. Therefore:

• We can drop the readAllItems() function in favor of a more general readArea(), and let the user implement the logic (we can have examples of how to do it)
• We implement a "helper class" with the API we have today, using the API offered by the new main endpoint class. Maybe we lose some optimization, but we can keep everything neatly separated.
• We keep the functions we have today, avoiding breaking the API. We include all logic to translate addresses and to keep a list of wanted addresses as we have today. We have more room for optimization, but it can be then a bit harder to organize everything.

All this is based on my own experience implementing other protocols, so it would be very nice to hear your feedback on it! What do you suggest?

[gfcittolin]

Hi everybody,

I've been working a bit more on this lately, and I'd like to ask you to take a look at the next branch, It's still pretty raw and still lacks some functionalities that the current version has, mainly the ability to translate the read data to javascript objects according with the string addresses.

Even though, It should be possible to establish and maintain a connection to a PLC and read data from and write data to it. There are currently "helper" functions like async readDB(db, address, length) or async readInputs(address, length) that return a Buffer with the read data.

One nice thing is the addition of error descriptions, so that, for example, if we try to call readDB() at an invalid address, the promise will be rejected with a self-explanatory error code, without hurting the connection itself.

Another big thing that you may find in the sources is the addition of support to MPI/PPI/DP connections. by using an USB-MPI adapter. Actually, most of it will be in a separate library that I'll be putting in another repo in the next few days, mainly because it will have native dependencies (due to USB).

You can check out a sample at /test/playground/test01.js. @plcpeople @sembaye I'd really appreciate your comments!

[gfcittolin]

Just created a Gitter room for more detailed discussions

[gfcittolin]

As mentioned above, please check out the mpi-s7 library we've been developing. I've just tested with 2 or 3 different adapters and s7-300 only. If any of you have an old MPI PLC lying around, I'd appreciate if you could test it.

[gfcittolin]

Hi everyone,

I'd say we've reached a "beta" stage of the 1.0 release (next branch). It has been completely rewritten, making room for upcoming functionalities and improvements, besides having a structured codebase, better documentation and unit testing included.

It should be feature-paired with the current release, but the API has drastically changed. As to make testing and migration easier, there's a "legacy" mode that has the same functions as today, but using the new code instead. You can try it by using require('nodes7/legacy') instead of the default require. Again, I'd like to hear your feedback.

@plcpeople depending on the feedback, we can try publishing this as 1.0.0~beta1. It would make easier for people to test, while still delivering the current version for anyone doing a normal npm install or anything depending on nodes7.