Don't set_result on completed futures.

[jameshilliard]

We need to guard self.response_future.set_result with a done() check to prevent asyncio.exceptions.InvalidStateError: invalid state errors.

Similar to #2581 I think we also need to flush the recv_buffer if the transaction is cancelled by the caller(say by the caller executing a transaction inside of another asyncio.wait_for that times out before the asyncio.wait_for inside of the execute call), in this case however we should re-raise the exception after the flush so that no retries are attempted.

Since self.response_future is cancelled in this case we should also guard the self.response_future.set_result call.

Should hopefully fix #2580 in combination with #2581.

fixes #2580

[janiversen]

see other PR.

[jameshilliard]

Ah, yeah, I guess if we purge the buffer before each send we don't need to clean up on exceptions.

[janiversen]

I think it would be more correct to notify the user by raising a ModbusIOException, because this is caused by a cancelation from outside pymodbus, and the app knows how to handle it correctly.

[jameshilliard]

notify the user by raising a ModbusIOException

If the future is already completed due to the future being cancelled I don't think we can set another exception like ModbusIOException on it. The execute function already raises a ModbusIOException after all retries are exhausted so I'm not sure we need to raise an exception from here.

this is caused by a cancelation from outside pymodbus

I'm not sure that's always the case, I think the pymodbus retry loop asyncio.wait_for also cancels the response_future when the timeout triggers.

If the app cancels the operation a ModbusIOException doesn't seem to make sense to me, on cancellation I think the normal CancelledError(which should AFAIU happen automatically from within the execute codepath waiting on the future even when cancelled externally by the app) is what is expected to be emitted since cancellation is not neccesarially indicative of any actual modbus error but may simply be the app abandoning the operation for whatever reason.

[janiversen]

I think we cannot just silently ignore a real error! if this happens it means that client handles 2 requests in the same call an that is. serious bug.

This is also correct if it happens for internal reasons, which I am pretty sure it does not.

[jameshilliard]

Maybe we should just log something? I mean if there isn't an active transaction I'm not sure what else we can do here as we don't have a future we can use to return an exception to right?

[jameshilliard]

This can also happen if a slave writes garbage on the line without a request right?

[janiversen]

The style in pymodbus is to raise a ModbuIOException, when something like this happens....typical users do not look in the log.

If a slave (server) writes data without being asked to, there are no request waiting, the loop is not active and data will be removed with next request.