RIOT-OS / RIOT

RIOT - The friendly OS for IoT
https://riot-os.org
GNU Lesser General Public License v2.1
4.9k stars 1.98k forks source link

Feature Request: ModBus RTU #13869

Open theltybc opened 4 years ago

theltybc commented 4 years ago

Description

ModBus is usefull protocol for comunicate beetwen sensors and MCU.

I am confused that there is no implementation for RIOT. Perhaps there are some difficulties in implementation this?

I`m ready to realize this. But I need consultation how to start.

theltybc commented 4 years ago

Im wana undestand this is need in RIOT or this better make in my own project(and dont show anyone).

benpicco commented 4 years ago

Sure, go ahead - nobody has been working on this yet. I don't know anything about ModBus RTU - is it used similar to I2C? It sure would be helpful to have this in RIOT.

MrKevinWeiss commented 4 years ago

This one I can actually help with. I used to do modbus stuff all the time and I have a setup at home.

I think it is something that can fit into RIOT quite well and would be willing to review and test.

Were you thinking of having modbus master, slave or both? Are you planning on using a package or implementing yourself?

theltybc commented 4 years ago

Need both. I think make like this https://github.com/smarmengol/Modbus-Master-Slave-for-Arduino.

But call poll all time not so good. Maybe use thread? master: on send request pass pid and then wait answer msg_receive(msg). slave: must set pid of thread who proccess modbus.

Both can get message/error. But slave must create and send answer himself.

Message to thread sent from callback uart_rx_cb_t. But if timeout happens how proccess it immediately? Otherwise timeout been detected only on receive next byte.

MrKevinWeiss commented 4 years ago

Maybe for the slave implementation:

The master should be a bit easier as you should already know all the information. You do need a timeout for the master as well, the question is should you stick to the each char has a timeout of 3.5 baud or can we relax it to reduce the context switch and say the whole message times out after 3.5 x baud x message_size?

theltybc commented 4 years ago

Mean on init slave create thread for them(3 modbus -> 3 thread)?

3.5 115200 20 = 8064000 I think in common case is too much.

and master work like this? this possible?

modbus_rtu_message_t * modbus_rtu_send_request(modbus_rtu_t *modbus, modbus_rtu_message_t *message) {
  modbus->_msg = message;
  modbus->_pid = thread_getpid();
  prepare_send_request(modbus);
  msg_t msg;
  if (xtimer_msg_receive_timeout(&msg, modbus->timeout) < 0){
    return NULL; // timeout
  } else {
    return message; // ok
  }
}
MrKevinWeiss commented 4 years ago

Mean on init slave create thread for them(3 modbus -> 3 thread)?

I guess that is what you would need.

3.5 115200 20 = 8064000

Sorry it should be 3.5 / baudrate = 3.5 / 115200 = 30us timeout per character

and master work like this?

Something like that, in that case the timeout would be 3.5 / baudrate sizeof(msg.content) = if you were expecting read 10 holding registers (10 2 + 3 + 2) then = 3.5 / 115200 * 25 = 760us

I would give that a try, TBH my modbus implementations have usually been bare metal or FreeRTOS but from a high level this makes sense.

theltybc commented 4 years ago

Beta version ready.

Now exist only Read registers and Write registers.

Example In example use stm32f103. Sent request to self and response to self

Workflow:

MrKevinWeiss commented 4 years ago

Nice job, I will give it a try on one of my boards. Maybe you can make a WIP PR so I can give some feedback?

theltybc commented 4 years ago

Most usedfull Read(0x3) and Write(0x10) Multiple registers function? Maybe, other functions turn on like:

#ifdef MODBUS_RTU_USE_COILS
//..
#endif // MODBUS_RTU_USE_COILS
MrKevinWeiss commented 4 years ago

The question is what level of resolution do we want? The best is if we can turn on/off each function or turn on all

so

#if defined(MODBUS_RTU_USE_READ_COILS) || defined(MODBUS_RTU_USE_ALL)
...
#endif /* MODBUS_RTU_USE_READ_COILS */
#if defined(MODBUS_RTU_USE_WRITE_COILS) || defined(MODBUS_RTU_USE_ALL)
...
#endif /* MODBUS_RTU_USE_WRITE_COILS */
#if defined(MODBUS_RTU_USE_READ_DI) || defined(MODBUS_RTU_USE_ALL)
...
#endif /* MODBUS_RTU_USE_READ_CDI */
...
theltybc commented 4 years ago

this so ugly

#if defined(MODBUS_RTU_USE_READ_COILS)
        case MB_FC_READ_COILS:
#endif  /* MODBUS_RTU_USE_READ_COILS */
#if defined(MODBUS_RTU_USE_READ_DISCRETE_INPUT)
        case MB_FC_READ_DISCRETE_INPUT:
#endif  /* MODBUS_RTU_USE_READ_DISCRETE_INPUT */
#if defined(MODBUS_RTU_USE_READ_REGISTERS)
        case MB_FC_READ_REGISTERS:
#endif  /* MODBUS_RTU_USE_READ_REGISTERS */
#if defined(MODBUS_RTU_USE_READ_INPUT_REGISTER)
        case MB_FC_READ_INPUT_REGISTER:
#endif  /* MODBUS_RTU_USE_READ_INPUT_REGISTER */
#if defined(MODBUS_RTU_USE_READ_COILS) || \
        defined(MODBUS_RTU_USE_READ_DISCRETE_INPUT) || \
        defined(MODBUS_RTU_USE_READ_REGISTERS) || \
        defined(MODBUS_RTU_USE_READ_INPUT_REGISTER)
            write_count(modbus);
            /* id + func + addr + count */
            modbus->size_buffer = 6;
            break;
#endif  /* MODBUS_RTU_USE_READ_COILS || MODBUS_RTU_USE_READ_DISCRETE_INPUT || MODBUS_RTU_USE_READ_REGISTERS || MODBUS_RTU_USE_READ_INPUT_REGISTER */
stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.