modbus_poll should change timeout->silent only for async packet

[Nick507]

The risk of current implementation is to miss timeout state in modbus_send_rtu, because second call of modbus_poll will change it to silent.
Currently it works, just because modbus_poll is called from modbus_send_rtu, but this is very dangerous - in other conditions modbus_poll can be called by core main thread, because it hooks grbl.on_execute_realtime.
And I think that intention was that modbus_poll procedure handles this state transition for async packet, whereas for blocking packet timeout state will be changed to silent/idle by modbus_send_rtu code.

Proposed fix:

Additionally, I didn't find on_rx_exception callback for timeout case for async packet.

[terjeio]

Currently it works, just because modbus_poll is called from modbus_send_rtu, but this is very dangerous - in other conditions modbus_poll can be called by core main thread, because it hooks grbl.on_execute_realtime.

The risk is, IMO, low since grblHAL in essence is single threaded. I'll add your test anyway since the state transition is also taken care of when the blocking loop exits.

Additionally, I didn't find on_rx_exception callback for timeout case for async packet.

This is deliberate, async packets is intended for querying the device status and is typically sent when the real time status is requested. But since the API may be used for other devices than VFDs is should be added? And perhaps support for async timeouts should be added to the VFD drivers too?

[Nick507]

This is deliberate, async packets is intended for querying the device status and is typically sent when the real time status is requested

My understanding was that async call differs from sync call just by non-blocking waiting for result, but result of operation is important and must be equal for async/sync from API perspective.
Using async in VFD to request current speed (when TO result is not important) is just a particular use case.
Anyway, I just pointed that, and I think it will be better to create a separate issue, if you agree.

[terjeio]

A new issue is ok - provinding the timeout callback is easy, how it should be handled is another story. Retry or not? Generate an alarm and in which circumstances? ...

[Nick507]

Is your concern about request speed case or in general?
I think that for request speed you can leave it as it is now - do not add any additional handling. Alarm will be raised after VFD_RETRIES according to current code.
In general - it depends on what user needs, this is not a question for modbus API.

[terjeio]

I did not add the async timeout callback since I did not (and still do not) have a clear idea how to handle it at the client side.
BTW my plan is to move the RTU code to the core to make the API generally available, not bound to the spindle plugin.