Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to implement request-response pattern? #132

Open
TonyBeswick opened this issue Oct 17, 2024 · 4 comments
Open

How to implement request-response pattern? #132

TonyBeswick opened this issue Oct 17, 2024 · 4 comments

Comments

@TonyBeswick
Copy link

Hi, I'm trying to implement a CAN device that receives ISOTP requests and responds back to the sender device. Its a non-automotive application, with a variable number of bus devices. Perhaps a little non-standard.

I'm using normal 29-bit addressing with a custom addressing scheme, which I'll simplify to:

  • bits[0-7]: sender id
  • bits[8-15]: destination id

It would be great if I could do something like:

import isotp
import can

class CustomAddress(isotp.Address):
    def __init__(my_id):
        # omitting detail here for brevity
        pass

bus = can.interface.Bus(bustype='virtual', channel='vcan0') # replace with real bus details

notifier = can.Notifier(bus, [])
isotp_stack = isotp.NotifierBasedCanStack(bus, notifier, address=CustomAddress(23))
isotp_stack.start()

while True:
    request = isotp_stack.recv(block=True) # assume request came from arbitration_id 0xxxxx0102
    # process response
    response = isotp_stack.send(response)  # send response to arbitration_id 0xxxxx0201 (sender and destination swapped)

The problem I have is that the arbitration id gets stripped out during the ISOTP Rx processing, so its not available to my # process response code.
And the txid is fixed for the stack at instantiation time, so a bit tricky to vary the arbitration id of the response based on the request.

What I'm looking at doing as a workaround is:

  • grab each CAN frame off the bus, recording its arbitration_id
  • filter the frame based on the arbitration_id (i.e. do the Address.is_for_me() logic)
  • send the frame to TransportLayerLogic._process_rx(msg)
  • check for any new data in TransportLayerLogic.rx_queue and associate it with the above arbitration_id.
  • do my response processing
  • create a new stack for the Tx path with the txid based on the arbitration_id from the Rx message (with sender and destination swapped).
  • send the response on this new stack

Questions

  • Any ideas on a cleaner way to implement such a request-response pattern?
  • Any interest in PRs that would help support this use case? (e.g. including arbitration id with received data, allowing specifying of custom txid with send data)

Any help/guidance much appreciated, thanks.

@pylessard
Copy link
Owner

pylessard commented Oct 17, 2024

Hi,
Yes, your addressing scheme is non-standard. If you are stuck with it, you can implement your own address object, it should work. Inherits AbstractAddress. Here are the methods and an insight about what you need to do

  • get_tx_extension_byte : Return always 0, will never be called
  • get_rx_extension_byte : Return always 0, will never be called
  • is_for_me : Your logic here
  • get_tx_arbitration_id : should be straightforward
  • get_rx_arbitration_id : should be straightforward
  • is_tx_29bits : return True
  • is_rx_29bits : return True
  • requires_tx_extension_byte : return False
  • requires_rx_extension_byte : return False
  • get_rx_prefix_size : Return 0
  • get_tx_payload_prefix : Return empty bytes() object
  • is_partial_address : return False
  • get_content_str : Return something that can be displayed in the console.

Once you have that, the rest should be transparent (hopefully).
No need to make 2 stacks by the way. Just use one.
I also suggest you use the NotifierBAsedCanStack as it uses a Notifier that does not deplete the bus receive queue.

An I wouldn't add support for a custom adressing scheme inside this repo, it is meant to support ISO-15765 and your adressing scheme is not part of it.

Hope that helps

@pylessard
Copy link
Owner

I would also need to change that line to check for AbstractAddress instead. I'm comfortable with that.
https://github.com/pylessard/python-can-isotp/blob/v2.x/isotp/protocol.py#L1202

@TonyBeswick
Copy link
Author

Thanks @pylessard!
If I find I need to inherit from AbstractAddress rather than Address, I'll come back with a PR for
https://github.com/pylessard/python-can-isotp/blob/v2.x/isotp/protocol.py#L1202

No need to make 2 stacks by the way. Just use one.

ah, yes I see trying to respond with a different stack was a dumb idea. The txid needs to be known prior to processing the Rx (for flow control messages). I'm thinking it would still be appropriate to have 1 stack per device I'm communicating with though, yes?

@pylessard
Copy link
Owner

Exact,
Yes 1 stack per device is the best

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants