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

Request timed up parameters as soon as possible #1136

Merged
merged 4 commits into from
Jan 16, 2019

Conversation

angri
Copy link
Contributor

@angri angri commented Dec 12, 2018

Current version of PX4 has almost 700 parameters. Pulling all of those at once even through serial connection @ 921600 time after time leads to timeout for some parameters (usually 3-5, sometimes about 30, depending on what else is going on on the link). Current implementation of params plugin rerequests missing parameters, but does it only on timeout event. So after the first timeout has passed params plugin re requests the first missing parameter and resets the timeout. Then the second missing parameters gets processed only when timeout happens again. It takes over 30 seconds to pull 30 missing parameters. During that time the service ~param/pull doesn't return.

Proposed solution makes params plugin rerequest the next missing parameter as soon as the previous one was delivered.

Avoid vaiting for the next timeout
Copy link
Member

@vooon vooon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem with that patch is that you broke request retry counter.

Present code gives 3 tries to request each parameter.
What you can do - lower timeout period if rx is timedout and you received expected param.

Other possible method - transmit requests in batches.

@vooon
Copy link
Member

vooon commented Dec 12, 2018

Hmm, that state machine is too complicated with all that class-level variables.

Perhaps much better if you will add mapping to save retry count for each parameter.
Then move retry code to method and reuse in both: timeout and msg handler.

@angri
Copy link
Contributor Author

angri commented Jan 14, 2019

Ok, so I found a bug in my code. The first timeout to happen was changing state to RXPARAM_TIMEDOUT, but that state was not checked for in the timeout callback itself. So now it's supposed to work this way: request parameter list and drop out of parameters_missing_idx all the parameters that were delivered. When first timeout happens plugin switches to RXPARAM_TIMEDOUT state and starts to request parameters one by one. Each of those still have 3 attempts (see line 492).

@vooon vooon added this to the Version 0.29 milestone Jan 16, 2019
@vooon vooon merged commit 72881f6 into mavlink:master Jan 16, 2019
@vooon
Copy link
Member

vooon commented Jan 16, 2019

Now looks better, LGTM. Thank you!

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

Successfully merging this pull request may close these issues.

None yet

2 participants