Add ecu_functinal_address and send_diagnostic_func for DoIPClient

[6reend0g]

Hi, jacobschaer,

When I use this DoIPClient to diagnose a real ECU, it seems that I can`t send a message with functional address of ECU while establishing the socket with physical address. To resolve this issue, I added a parameter and function to the DoIPClient.

Feel free to suggest any edits or alternative solutions.

Have a good day!

[jacobschaer]

So - you call send_diagnostic_func() manually? And, as far as I can tell, that function is exaclty the same as send_diagnostic() but with the functional address instead of the logical address?

Is this not equivalent to just setting the logical_address to the functional address in the constructor? I would assume that your ECU would respond to the functional address for the duration of the session so no need to even use the logical address? The only use case I could think of is if maybe the activation request requires the logical for some reason but you want to use functional for the diagnostic sessions. In that case, perhaps we just need a helper to let you swap _ecu_logical_address after construction? Though what that would actually look like from a DoIP standpoint I'm not sure.

send_diagnostic() is normally used by the UDS adapter which wouldn't be aware of your function.

[6reend0g]

Thanks for your reply,

Currently, the DoIPClient uses ecu_logical_address as the TA(target address) in messages sent by send_diagnostic(), which means it can only communicate with a single logical address of a single DoIP node. However, in some cases, we have multiple logical address - for example, as I mentioned earlier, when the physical and functional addresses are implemented over TCP ... or when we need to route certain DoIP messages to a sub-node.

So, to handle this, we need a function to send Diagnostic message or even DoIP message with a customizable TA, maybe we can add a parameter for send_diagnostic() which can define the TA instead add another function. One option is to add a parameter to send_diagnostic() to specify the TA instead of creating a separate function. However, this might introduce some troubles for the connector.

Maybe you are right, swapping _ecu_logical_address after construction could be a viable solution, but I think the _ecu_logical_address represents the property of the client-server model...okey, any way, don't mind my overthinking.

If you think swap the _ecu_logical_address after construction is acceptable, just let me know, and I’ll modify the commit and submit a new PR.

Have a good day.

[6reend0g]

This feature might not be reachable in the udsoncan, as it is primarily designed for CAN where creating two, three, or even more clients is straightforward,howerver, it`s much harder for a DoIP server and client to establish multiple TCP connection. I don not have a deep understanding of udsoncan, I just think we need a enhanced method to send diagnostic messages in the context of DoIP adoption.

[jacobschaer]

So, you're manually framing your own diagnostic requests and not using a uds library? And you want to be able to send them to other addresses within a given TCP connection?

I'm not a big fan of copy/pasting that function - there's a lot of "heavy lifting" from the function that's copied and you're only changing one thing. I think the correct design here is

Create a new function
send_diagnostic_to_address(address, ...):

Which takes all the guts out of send_diagnostic(). Then have send_diagnostic() call that new function with self._ecu_logical_address . Then you're free to call send_diagnostic_to_address() directly with the functional address or whatever you want. No copy/pasting, no new member variables, no breaking API changes. Seems clean to me.

The unit tests should continue to cover it that way too

[6reend0g]

Hi, I have updated my PR. Please review it and let me know if it meets your requirements.

[jacobschaer]

Looks good - can you please rebase to get the latest CI? It seems Github's ubuntu-latest image dropped support for 3.7

[6reend0g]

Got it! Please try the UT again.

[jacobschaer]

Looks good let me see about cutting a new release