Fix controller sorting issue while loading large number of controllers #1180
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
requested review from
bmagyar,
destogl,
erickisos,
harderthan,
mcbed,
progtologist and
VanshGehlot
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1180 +/- ##
=======================================
Coverage 47.63% 47.63%
=======================================
Files 40 40
Lines 3441 3445 +4
Branches 1864 1866 +2
=======================================
+ Hits 1639 1641 +2
Misses 480 480
- Partials 1322 1324 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
bmagyar
added
backport-humble
… controller used by a couple of controllers
saikishor
force-pushed
the
testing_large_number_of_chained_controllers
branch
from
fca37fc
to
2e32546
Compare
bmagyar
reviewed
Nov 29, 2023
| /// The simulated controller chain is like every joint has its own controller exposing interfaces | ||
| /// and then a controller chain using those interfaces | ||
| /// | ||
| /// There are 30 more other basic controllers to check for crashing |
There was a problem hiding this comment.
I think we are missing the 30 additional controllers or I've missed it during review
There was a problem hiding this comment.
Lol! You are right, let me reformulate it and add some more controllers for more complexity
There was a problem hiding this comment.
Done. Thank you for the comments :)
…er_tree test + renaming variables
bmagyar
approved these changes
Dec 3, 2023
This was referenced Dec 3, 2023
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to solve the issue mentioned by @GilmarCorreia : #1170. With the tests, I'm able to reproduce the error at some number around 20 controllers, so in order to make it robust, I've added 2 more different tests, testing with around 200+ controllers and also trying to simulate complex chaining that could be in day-to-day use-cases.