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

Round up for actuator loop period #77

Merged
merged 2 commits into from
Feb 28, 2023
Merged

Conversation

davidinkyu-kim
Copy link
Contributor

Elmo actuator was not working correctly(position tracking error when profiled traj is commanded) with loop rate of power of 2.

While converting target_loop_rate into egd configuration which is INT8 type, round up should be done.

Tested with 128hz, 256hz and working as expected without error.

@alex-brinkman
Copy link
Contributor

It's unclear to me why this would solve anything. Checkout this compiler explorer example to see the difference between round and not-rounded results: https://godbolt.org/z/cdf39P87n

Mind helping me understand why this works?

@alex-brinkman
Copy link
Contributor

This does solve a bug for rates above 2000hz as 0.0005 sec in msec would round down to zero in the current implementation

@davidinkyu-kim
Copy link
Contributor Author

loop_rate is converted into jsd_egd_config_t/loop_period_ms which is int8_t type: https://github.com/nasa-jpl/jsd/blob/master/src/jsd_egd_types.h#L249

Without round up, for example of loop rate 128hz, loop_period_ms is set to 7ms. And fcat runs at 7.8126ms period which is slower than the Elmo control rate. When Elmo's host control loop(0x60C2 ) is set faster than actual, Elmo cause a position tracking error: Reference to the figure: https://fornat1.jpl.nasa.gov/sts/sta/clark/bringup/-/issues/16#note_25908

With the roundup, host control loop rate is set at '8ms' in case of fcat@128hz, which is slower than actual control loop. Elmo works okay in that setup.

And for 2000hz case, anything faster than 1000hz the loop_period_ms will be set to 1: https://github.com/nasa-jpl/jsd/blob/master/src/jsd_egd.c#L804

@alex-brinkman
Copy link
Contributor

ahhh very nuianced interaction! Can you put in a comment describing the bug and why it needs to round up before you merge and close out this issue?
You have my full approval, great sleuthing!

@davidinkyu-kim davidinkyu-kim merged commit 76165d8 into master Feb 28, 2023
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 this pull request may close these issues.

2 participants