PID demo revised by kngwyu · Pull Request #49 · oist/smartphone-robot-android

kngwyu · 2025-09-06T07:50:19Z

No description provided.

kngwyu · 2025-09-06T07:51:27Z

+<resources xmlns:tools="http://schemas.android.com/tools">
    <!-- Base application theme. -->
-    <style name="AppTheme" parent="Theme.AppCompat.Light.DarkActionBar">
+    <style name="Theme.PIDBalancer" parent="Theme.Material3.DayNight.NoActionBar">


this is actually very important to make the whole things work

I'm not sure I understand why this is important. It looks like you just changed the layout theme.

Because the slider only exists in the material theme, as far as I know.

kngwyu · 2025-09-06T07:52:00Z

        // error betweeen actual and desired wheel speed (default 0)
        double e_w = 0.0 - speedL;
-        Log.v(TAG, "speedL:" + speedL);
+        Log.v(TAG, "speedL: " + speedL + ", deg: " + thetaDeg);


@topherbuckley Can I ask what is the unit of speed here?

TLDR

It appears it is in mm/s, but does not account for slippage, or other non-linear effects. This is linear speed not angular. The radius of the wheel is accounted for.

Details

I don't recall off the top of my head, but looking here it is hardcoded to the value wheelSpeedExpAvgL

which you can follow to here

keep following tohere

Following to here

It appears it is in mm/s. I believe the division by 1000000000f is to convert nanoseconds to seconds, which is what is output from whatever method I was using for getting the timestamps. See more info at the top definitions of the wheelData class.

Also note I'm only accounting for a single (Left) wheel in this, so any drift between wheels is not accounted for. And finally noting that this is one of several "software" metrics I made for the speed. You can also try setting this to wheelSpeedBufferedL or adjusting the exponential filter or buffer size using the builders here if the control signal is not stable. I recall tuning this a fair bit though so I hope the default values work well enough. (I haven't checked this in several years though so I could be wrong). Ideally you use the builders so you can tune the values for whatever your application is (e.g. PID balancer). The hardcoded values, I hope are fine for most controllers, which is why I set them as the defaults.

I'm not sure if they still work or not, but I was using the scripts in utils to plot these kinds of data streams in real time to check for stability and whatnot.

Thank you! I'm gonna try the buffered one with default params anyways

topherbuckley

The big issue with this PR is that you have a rename of a directory in the same commit as changes to the files. Apparently git got confused and thinks there are a bunch of new files, when there are not. See my comment on apps/pidBalancer/src/main/java/jp/oist/abcvlib/pidbalancer/MainActivity.java for potential workarounds.

topherbuckley · 2025-09-06T21:14:38Z

Your PR is labeled as PID demo revised. Can you limit your changes to the PID demo then? I'm happy to take a look at these updates to the QR code demos if you think they are needed, but in a separate PR please.

topherbuckley · 2025-09-06T21:19:51Z

Why is this showing as a new file? Didn't it exist before? I'd like to see the diff if so. Normally git should detect a file move like this. Not sure what happened.

I see now. You changed the path from pidbalancer to pidBalancer. Can you:

Create a new branch based on main

Change the directory to pidBalancer without any other changes.

Commit to new branch

Rebase this branch onto that new branch.

Update the PR to reference the new branch

That should allow us to see the diff of what was actually updated.

If that doesn't work, just a PR with the directory change and nothing else, then we can merge that to main and revist this PR. Otherwise everything looks like new additions and your updates are indiscernible.

Sure I'm gonna re-organize the PR

topherbuckley · 2025-09-06T21:38:31Z

        // error betweeen actual and desired wheel speed (default 0)
        double e_w = 0.0 - speedL;
-        Log.v(TAG, "speedL:" + speedL);
+        Log.v(TAG, "speedL: " + speedL + ", deg: " + thetaDeg);


TLDR

It appears it is in mm/s, but does not account for slippage, or other non-linear effects. This is linear speed not angular. The radius of the wheel is accounted for.

Details

I don't recall off the top of my head, but looking here it is hardcoded to the value wheelSpeedExpAvgL

which you can follow to here

keep following tohere

Following to here

It appears it is in mm/s. I believe the division by 1000000000f is to convert nanoseconds to seconds, which is what is output from whatever method I was using for getting the timestamps. See more info at the top definitions of the wheelData class.

topherbuckley · 2025-09-06T21:45:48Z

        // error betweeen actual and desired wheel speed (default 0)
        double e_w = 0.0 - speedL;
-        Log.v(TAG, "speedL:" + speedL);
+        Log.v(TAG, "speedL: " + speedL + ", deg: " + thetaDeg);


Also note I'm only accounting for a single (Left) wheel in this, so any drift between wheels is not accounted for. And finally noting that this is one of several "software" metrics I made for the speed. You can also try setting this to wheelSpeedBufferedL or adjusting the exponential filter or buffer size using the builders here if the control signal is not stable. I recall tuning this a fair bit though so I hope the default values work well enough. (I haven't checked this in several years though so I could be wrong). Ideally you use the builders so you can tune the values for whatever your application is (e.g. PID balancer). The hardcoded values, I hope are fine for most controllers, which is why I set them as the defaults.

topherbuckley · 2025-09-06T21:47:31Z

        // error betweeen actual and desired wheel speed (default 0)
        double e_w = 0.0 - speedL;
-        Log.v(TAG, "speedL:" + speedL);
+        Log.v(TAG, "speedL: " + speedL + ", deg: " + thetaDeg);


I'm not sure if they still work or not, but I was using the scripts in utils to plot these kinds of data streams in real time to check for stability and whatnot.

topherbuckley · 2025-09-06T21:52:26Z

Also be sure if passes the build test (it currently does not) before merging to main. You are free to merge it to a dev branch in the meantime if you want it on the oist repo rather than your fork some reason though.

kngwyu · 2025-09-08T03:36:25Z

Plz understand that I have very limited time for this 😢
I need to write thesis. But anyways I'm gonna re-organize everything

kngwyu · 2025-09-08T03:42:27Z

I'm a bit urgent to prepare demo so I'm gonna keep this branch as is, but I'll prepare PRs that picks up a related chunk of changes in a stream-lined way

kngwyu added 2 commits September 6, 2025 16:46

PID demo revised

1c1d118

Unused imports

8744778

kngwyu commented Sep 6, 2025

View reviewed changes

topherbuckley requested changes Sep 6, 2025

View reviewed changes

kngwyu closed this Sep 8, 2025

Conversation

kngwyu commented Sep 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

TLDR

Details

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

topherbuckley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

TLDR

Details

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

topherbuckley commented Sep 6, 2025

Uh oh!

kngwyu commented Sep 8, 2025

Uh oh!

kngwyu commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kngwyu commented Sep 8, 2025 •

edited

Loading