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

Rate limit the sending of consecutive frames regardless of server request #61

Closed
dpatel20 opened this issue Feb 6, 2022 · 9 comments · Fixed by #62
Closed

Rate limit the sending of consecutive frames regardless of server request #61

dpatel20 opened this issue Feb 6, 2022 · 9 comments · Fixed by #62
Assignees

Comments

@dpatel20
Copy link

dpatel20 commented Feb 6, 2022

I am on Windows using Vector HW. I use udsoncan to perform a transfer data operation in 4KB blocks with python-can-isotp as the TP layer.
The server, in its flow control, has stmin=0 so python-can-isotp sends as fast as possible. However, it seems the Vector driver sometimes can't handle the fast rate and throws a XL_ERR_QUEUE_IS_FULL error.

According to this page:

You have a program that writes messages very quickly to the buffer so that the data rate of the real bus is exceeded (e.g. while-/for-loops).

Suggesting that the bus data rate needs to be respected to avoid this error.

Therefore, I'd like to rate limit the transmit. Is there a way to have python-can-isotp set a minimum value for transmit stmin? i.e. if server gives stmin=0, we still limit it to some value (e.g. 1ms separation)? Perhaps based on the bus rate?

@pylessard
Copy link
Owner

That's a good point. I could allow override of stmin. But even better, we could specify the internal buffer size. Like that it would be possible to do a rate limiter without pausing betwern each consecutive frame.

Will have a look

@pylessard pylessard self-assigned this Feb 6, 2022
@pylessard
Copy link
Owner

pylessard commented Feb 14, 2022

Hi @dpatel20
Can You give a try to the branch named support-rate-limiter

Here's the documentation
image

Set rate_limit_enable = True

Then
The product of rate_limit_max_bitrate and rate_limit_window_size will define the maximum burst size the driver will need to withstand. I suggest you to reduces rate_limit_max_bitrate until everything works as expected

I will merge once you validate that this fixes your problem

@dpatel20
Copy link
Author

Hi -

Thanks for the new feature! I have tested it and can confirm it works well. I modify the rate_limit_max_bitrate and it does indeed limit the throughput and I no longer get buffer full errors.

I tried it with a very high busload too and was able to drop the rate_limit_max_bitrate and still get it to work.

pylessard added a commit that referenced this issue Feb 14, 2022
@pylessard
Copy link
Owner

Merged. Will be included in next release.
Thank you for your feedback!

@pylessard
Copy link
Owner

Documentation is also updated
https://can-isotp.readthedocs.io/en/latest/isotp/implementation.html#rate-limiter

@notmmao
Copy link

notmmao commented Jun 16, 2022

I'm not sure if I should reopen this issue.

#62
When a TRANSMIT_FF_STANDBY msg is determined to be sent, the tx_state should be changed to WAIT_FC instead of IDLE.
Otherwise I always get error:
Received a FlowControl message while transmission was Idle. Ignoring

my params:
{'tx_padding': 0, 'blocksize': 0, 'rate_limit_enable': True, 'rate_limit_max_bitrate': 64000, 'rate_limit_window_size': 0.001}

@pylessard
Copy link
Owner

Oh, that looks like a bug indeed.
TRANSMIT_SF_STANDBY --> Should go to IDLE when unlocked
TRANSMIT_FF_STANDBY --> Should go to WAIT_FC when unlocked

@pylessard
Copy link
Owner

I forgot about this one. Reopening to fix the bug raised by @notmmao

@pylessard
Copy link
Owner

I fixed this.

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

Successfully merging a pull request may close this issue.

3 participants