Conversation
Enet4
left a comment
There was a problem hiding this comment.
However I'm not very clear on what's going on exactly, as the functions they use seem to return
Err(ConnectionClosed)in some instances, which doesn't seem to affect the test.
These tests were made to cover the unlikely scenario of an SCU sending more data than it should during the A-ASSOCIATE handshake. This does not seem to be a critical change in behavior, though it would be nice to understand why those results had to be ignored.
|
I'm pushing a change that fixes the other FIXME. On receipt of an A-ASSOCIATE-RQ, the peer is expected to send an A-ASSOCIATE-RP, as the other peer is waiting for it, but none was being sent, which resulted in the latter seeing a connection shutdown instead of the expected A-ASSOCIATE-RP. I think that makes the patch be in a mergeable state. |
Enet4
left a comment
There was a problem hiding this comment.
This is now considered acceptable. Despite the significant change in behavior, it is safe to say that this is well preferred over the current state. Thank you again!
Implements point 1 of #714 (comment). There was a unit test that seemed to rely on packets being sent upon dropping the association object, so I had to fix those too. However I'm not very clear on what's going on exactly, as the functions they use seem to return
Err(ConnectionClosed)in some instances, which doesn't seem to affect the test.If all is OK, I will remove the FIXME's.