christoph2 / pyxcp

ASAM XCP in Python
http://pyxcp.rtfd.org
GNU Lesser General Public License v3.0
197 stars 63 forks source link

DISCONNECT master command does never timeout but retries indefinitely #108

Closed tttech-miori closed 1 year ago

tttech-miori commented 1 year ago

DISCONNECT master command does never timeout but retries indefinitely.

The standard says that DISCONNECT shall be retried max 2 times before giving up; SYNCH shall also be issued before as a pre-action.

So, I'd expect: DISCONNECT -> disconnect timeout -> SYNC -> SYNC timeout -> SYNC timeout -> raise Timeout Exception. Or, alternatively: DISCONNECT -> disconnect timeout (retry=1) -> SYNC -> SYNC timeout (rety=1) -> SYNC timeout(retry=0) -> DISCONNECT -> disconnect timeout (retry=0) -> SYNC -> SYNC timeout (retry=1) -> SYNC timeout (retry=0) -> finally, raise Timeout Exception for DISCONNECT.

Please be aware that I am still not aware of many little details of the XCP protocol, so please correct me if I'm wrong.

Use-case: disconnect a slave which is potentially connected, just as a matter of fact to reset communication from a known state.

EDIT: as a hint, I believe there is some strange behavior in the Executor().call class and method. In particular, self.repeater is never ever invoked due to self.error_code never being set to None or so. EDIT2: what I'm seeing on the canbus is the following sequence of master -> slave frames: FE -> 2seconds -> FC -> 2 seconds -> FC -> 2 seconds -> FC -> 2 seconds -> FC .... forever. The slave doesn't answer because it is already disconnected.

tttech-miori commented 1 year ago

version 0.18.48

tttech-miori commented 1 year ago

Probably fixed here: https://github.com/christoph2/pyxcp/pull/120 To be tested.

tttech-miori commented 1 year ago

This is fixed by https://github.com/christoph2/pyxcp/pull/120 Thanks!

christoph2 commented 1 year ago

Still not perfect -- I've tested the TIMEOUT case with my own cxcp project, there's no infinite loop anymore, but only a single SYNCH/DISCONNECT sequence.

Use-case: disconnect a slave which is potentially connected, just as a matter of fact to reset communication from a known state.

Not sure what potentially in this context should mean -- Or to put it another way: If there was a CONNECT before, the XCP slave must respond to DISCONNECT...

tttech-miori commented 1 year ago

Thank you for the reply.

Not sure what potentially in this context should mean -- Or to put it another way: If there was a CONNECT before, the XCP slave must respond to DISCONNECT...

True. At the same time, though, the action of disconnecting from a slave may be a preventive measure when you are probing system where its state is unknown i.e. I have my test-system / diagnostics tool and I issue a preventive "DISCONNECT" before attempting a new connection. In this situation, the slave is not going to answeron DISCONNECT. "Potentially" referred to the possibility of having a slave that is still in connected state regardless of pyxcp (e.g. diagnostic tool or test system did not properly shutdown xcp)

Still not perfect -- I've tested the TIMEOUT case with my own cxcp project, there's no infinite loop anymore, but only a single SYNCH/DISCONNECT sequence.

Yes, actually you are right, I was just ok the software to not indefinitely block :-). We should try that 2 times I guess based on the specification: DISCONNECT -> timeout t1 -> SYNCH -> timeout t1 -> timeout t1 -> DISCONNECT -> timeout t1 -> SYNCH -> timeout t1 -> timeout t1 -> END

NOTE: my slave runs the Vector XCP Basic implementation.

christoph2 commented 1 year ago

OK, I see.

To support such scenarios, I've added a new config-option (Commit d0bc55d23975ba0a6ca1c81a3888a1d8390cce32 , Release 0.18.59)

TOML

DISCONNECT_RESPONSE_OPTIONAL = true 

JSON

"DISCONNECT_RESPONSE_OPTIONAL": true