Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

Add theoretical mode #374

Merged
merged 1 commit into from
Feb 21, 2022
Merged

Add theoretical mode #374

merged 1 commit into from
Feb 21, 2022

Conversation

Oblarg
Copy link
Contributor

@Oblarg Oblarg commented Feb 19, 2022

Replaces the "waiting for data json" state with a theoretical analysis mode that allows you to plug in your own calculated feedforward gains for use with LQR. Also refactors the state machine to get rid of states that do nothing but call code and transition to other states (these are now just subroutines).

@calcmogul
Copy link
Member

The input boxes don't fit.
Screenshot_20220218_193448

@Oblarg
Copy link
Contributor Author

Oblarg commented Feb 19, 2022

Should be fixed. It'd be nice if I could reliably reset the formatting to default - we should have a button for that.

@calcmogul
Copy link
Member

calcmogul commented Feb 19, 2022

The response timescale box is missing from the JSON analysis. I'd expect the non-JSON box layout to just be a subset of the JSON box layout.

@Piphi5
Copy link
Collaborator

Piphi5 commented Feb 19, 2022

Have you tested the data loading part (seems to get blocked on plotting graphs)? For some reason sysid just freezes for me when I try to load data. My computer has been acting weird recently so that might be the issue, not the code itself.

@Piphi5
Copy link
Collaborator

Piphi5 commented Feb 19, 2022

It seems like data errors causes none of the graphs to be plotted.
image

@Oblarg
Copy link
Contributor Author

Oblarg commented Feb 19, 2022

That is correct; I can change that if you want.

@Piphi5
Copy link
Collaborator

Piphi5 commented Feb 19, 2022

Yeah ideally they can see the raw data so they can fix the motion threshold

Copy link
Collaborator

@Piphi5 Piphi5 left a comment

Choose a reason for hiding this comment

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

One concern is that the weird value update bug happens if we load the file the second time:
I haven't checked if this happens on main but it's probably some logic we can fix on this PR.
https://user-images.githubusercontent.com/29788153/154989342-ba430192-55ff-410d-96d8-2f640336ca10.mp4

Edit: The video came out blurrier than I expected but essentially the first time around the calculated gain is at 2.0879 and then the second time around it starts at 2.0879 and then pressing on convert gains to encoder counts turns it into 2.7182

@Oblarg
Copy link
Contributor Author

Oblarg commented Feb 21, 2022

One concern is that the weird value update bug happens if we load the file the second time: I haven't checked if this happens on main but it's probably some logic we can fix on this PR. https://user-images.githubusercontent.com/29788153/154989342-ba430192-55ff-410d-96d8-2f640336ca10.mp4

This happens in main immediately upon file load; I'm not sure why loading it twice would cause issues, though...

@Piphi5
Copy link
Collaborator

Piphi5 commented Feb 21, 2022

This happens in main immediately upon file load; I'm not sure why loading it twice would cause issues, though...

For some reason, I can't reproduce it on main. Sounds like some sort of UB then.

@Oblarg
Copy link
Contributor Author

Oblarg commented Feb 21, 2022

I'm having a hard time replicating the bug at all, even on this branch - I think something might be funky here.

@Oblarg
Copy link
Contributor Author

Oblarg commented Feb 21, 2022

Found the offending code, and it's fixed. Should be good now.

@Piphi5
Copy link
Collaborator

Piphi5 commented Feb 21, 2022

what was the error?

@Oblarg
Copy link
Contributor Author

Oblarg commented Feb 21, 2022

Failing to early exit out of the render function in a data error state if the reset button is pressed.

@Piphi5
Copy link
Collaborator

Piphi5 commented Feb 21, 2022

The error still shows up for me :/

@Oblarg
Copy link
Contributor Author

Oblarg commented Feb 21, 2022

What dataset is causing it? I cannot replicate it.

@Piphi5
Copy link
Collaborator

Piphi5 commented Feb 21, 2022

The original one linked on the discord. I'm assuming it might be related to the convert to encoder gains checkbox.

@calcmogul
Copy link
Member

The original one linked on the discord.

It would be good to upload it here.

@Piphi5
Copy link
Collaborator

Piphi5 commented Feb 21, 2022

@calcmogul calcmogul merged commit 0c1af2f into wpilibsuite:main Feb 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants