mycroes/Sally7

Extensions for reading/writing plain old byte arrays

Closed this issue · 3 comments

For my Sally7 wrapper, I added some extension methods to just read/write a byte array as a single data item per request.

I also plan to add some magic to automatically split the byte array into smaller chunks that fit into the maximum length of a single request (PDU related), so that I can seamlessly support very large DBs in our product.

@mycroes Do you think Sally7 would benefit by having such extensions available, or do you think this is a slippery slope adding things too far away from the core competencies of Sally7? Similar to all the barely-maintained stuff in S7NetPlus which is not just reading/writing data items :-)

I'm not sure about this one... I guess you're just creating a DataItem on the fly for the byte array, while for absolute performance you should keep your DataItems around. I do want to add address parsing (already did that for Sally7 in PlcMonitor), combined reads and read message caching, because in the read-many-write-sometimes case that's the biggest win, probably. Sally7 should mostly be about performance, I want to avoid opinionated convenience, because opinions differ.

Hmm I haven't thought about reusing the data item at all. Guess I do not really think with such a performance focused mindset, at least in C#/enterprise application development. I also probably have too large of an expectation that items which would live on the stack in other languages don't incur such a large overhead, even if I know that this is not the case here.

I'll probably still go the route of convenience for myself, even if that leaves some performance on the table. I would say anyone fully grasping all the implications is definitely able to adapt core Sally to their needs.

Btw. I explicitly said extensions because I could imagine such things being available in a separate assembly, so that it does not cause any unnecessary bloat and complexity on the core library. And additional code always adds some maintenance burden.

I understood the extension idea, I'll keep it in my thoughts either way. Performance shouldn't really be impacted by creating small classes like DataItem (although there's some overhead in creating DataItems for resolving the serializer and deserializer). We do perform a lot of reads, so if we don't waste memory that's a benefit. For the PlcMonitor that currently only reads when you click a button, it doesn't matter at all...

Anyway, I like your ideas and contributions, so please keep them coming 😉