debevv / nanoMODBUS

A compact MODBUS RTU/TCP C library for embedded/microcontrollers
MIT License
282 stars 58 forks source link

Support buffer-oriented callbacks #7

Closed jonathangjertsen closed 2 years ago

jonathangjertsen commented 2 years ago

If the API supported buffer-oriented callbacks (int write_buffer(const uint8_t *buffer, size_t size, int32_t timeout_ms, void *arg) and int read_buffer(uint8_t *buf, size_t size, int32_t timeout_ms, void *arg)), it would be easier to take advantage of hardware FIFOs and DMAs to reduce CPU load.

I worked around the byte-oriented API by having the write_byte and read_byte callbacks read/write into an intermediate buffer and performing the actual transfer once the correct number of bytes had been requested. This works but is a little clunky and is less efficient than a zero-copy transfer of the buffer that already exists in nmbs->msg.buf.

debevv commented 2 years ago

Yes, I thought about it when writing the library, but at the end I opted for keeping the interface and the internal code simple, leaving the choice of buffering (albeit in a less efficient way) to the end user. Plus, in a typical modbus application you are not going to move a lot of bytes anyway, so I don't think the overhead should be that impactful. Anyway, If implementing these 2 platform callbacks does not upset the library too much, I would be happy to receive a PR of this feature

jonathangjertsen commented 2 years ago

OK, I have an idea of how it might be implemented so I will give it a shot

jonathangjertsen commented 2 years ago

Here is my idea. It works in my tests (it's inefficient due to several calls to recv, but that can be improved separately). If it looks like an acceptable approach I'll make a PR which also updates the docs

https://github.com/jonathangjertsen/nanoMODBUS/commit/b8129ff439559933c51ab92176f8311fc5c6bf89

debevv commented 2 years ago

A the end I bit the bullet and decided to migrate the read/write interface to multi-byte functions only, inspired by your code. You can find the code + docs in the multibyte_platform_funcs branch.
Let me know what you think of it, as an user of the library. And thanks for your input

jonathangjertsen commented 2 years ago

Looks very good! This would let me remove a couple hundreds of lines of wrapper code. It would also be more robust since the wrapper doesn't need to anticipate the number of times the nanomodbus library will call the read_byte/write_back callback anymore.

debevv commented 2 years ago

Fine, closing this with 7e6ce4f4fdc767d31b65c5edec6e693821a37abd