-
Notifications
You must be signed in to change notification settings - Fork 5
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
Added Approach script #17
base: master
Are you sure you want to change the base?
Conversation
I have taken a look at the commit where the tests complained about too many lines. Also running
(Note the $ at the end of the last line). Tl;dr: everything works as it should work, but some editors dont show trailing LFs, leading to confusion What editor are you using to write the script? |
Without Rangefinders reporting actual distances as a result of FCUForward input, this feature cannot be tested. Testing the PID loop logic seems doable though, feel free to add if possible
Hi, I just used the Github web interface ;-) |
Well, now the checks are complaining that your code has no tests. |
We discussed this in Discord and concluded there's not a great way to test this at the moment - you'd need some kind of test harness to reduce distance as "time" passed. What's the correct way to submit a script with no tests? |
I don't really understand what the issue here is. At least some basic cases should be able to be tested just fine. There is no need to check every single edge-case. Just add some basic ones to make sure nothing is completely broken. |
How would I be able to run the tests locally so I don't have to force a commit just have a test run? Reason being I don't know what the exact output of the PID loop is supposed to be, is there a way to have a test condition that uses less then or greater than? As in |
Exactly, the testing is done using the yodk, which you can use to run tests locally. |
First attempt to add a PID controlled approach script