Skip to content

Conversation

@anirudhupadhyaya
Copy link
Contributor

No description provided.

@npetersen2
Copy link

@anirudhupadhyaya requests that I review this PR in terms of direction of content

@npetersen2
Copy link

@anirudhupadhyaya this is on the right track and seems to be written to the right audience, i.e., a user of the AMDC who is trying to interface an encoder to their control system.

There are a few typos and awkward sentences which should be resolved.

However, my main feedback would be to add a "master" block diagram which shows all the possible steps in processing the encoder integer feedback value into something useful for the control system. Then, replace your C code blocks with a single block of C code which implements the block diagram for "all cases" i.e., CW or CCW, etc.

I am not sure about your while() loop for performing the mod(x, 2*pi) operation. Probably better to just use mod() here unless you have a good reason not to use it.

Also, add a section about validating the encoder offset via the d-axis voltage during operation.

I am also curious to see your section on converting position to speed.

@anirudhupadhyaya anirudhupadhyaya marked this pull request as ready for review June 26, 2024 05:29

Choose a reason for hiding this comment

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

Thanks for adding this image, but this is not what I envisioned.

Can you change this to a block diagram which is like what you would find in simulink? i.e., a signal flows from left to right and is manipulated? This block diagram should match your C code implementation, just in a graphical form.

@anirudhupadhyaya
Copy link
Contributor Author

I am yet to update the block diagram, but I have addressed rest of the comments.

@anirudhupadhyaya
Copy link
Contributor Author

Block diagram updated.

Copy link
Contributor

@elsevers elsevers left a comment

Choose a reason for hiding this comment

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

Thanks @anirudhupadhyaya. I think this is on the right track. I have left you a few improvement requests below, and have gone ahead and implemented most of the needed changes as suggestions that you can simply accept.

In addition to this, I would like to see you add one (or more) reference papers to help users implement and think about the observer approach. There must be some good material from our digital control study group that Nathan and I did with you and Aravind that you can cite here?

You'll get big bonus points from me if you can include a MATLAB / Simulink example of a motor with an observer. I'd really love it if you can do this, but maybe too late in the game for this though.

anirudhupadhyaya and others added 2 commits June 30, 2024 12:23
Co-authored-by: Eric Severson <eric.severson@wisc.edu>
@anirudhupadhyaya
Copy link
Contributor Author

@elsevers I have addressed your review comments.

Copy link
Contributor

@elsevers elsevers left a comment

Choose a reason for hiding this comment

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

Hmm, something seems to have gone wrong here.

image

We're missing the discussion on how the low pass filter always producing a lagging estimate. I remember suggesting changes to this --> how 10 Hz is a good starting bandwidth, that it will always produce a lagging estimate, etc.

I can't seem to find the text I had edited for this. Do you think you accidentally rejected my suggestion?

@anirudhupadhyaya
Copy link
Contributor Author

Oh.. I guess it went away when I accepted the suggestions?

@anirudhupadhyaya
Copy link
Contributor Author

@elsevers I added a line on the bandwidth recommendation for low pass filter.

Co-authored-by: Eric Severson <eric.severson@wisc.edu>
@anirudhupadhyaya anirudhupadhyaya requested a review from elsevers July 1, 2024 00:30
elsevers and others added 28 commits June 8, 2025 11:17
Apply feedback from Professor's comment (option 2) to revise the steps to find offset
Update the image
Add the formula
Co-authored-by: Takahiro <114006024+noguchi-takahiro@users.noreply.github.com>
Update the variable names
Added detailed explanation and equations for determining encoder offset using closed-loop control and voltage measurements.
Added clarification about electrical angular velocity in the voltage equation.
Corrected the formatting of the voltage vector equation in complex vector form.
Clarified the conditions for estimating encoder offset by refining language regarding the alignment of the gamma-delta and d-q frames.
Updated notation for current commands in the equation for v_gamma.
Updated explanations regarding the calculation of the motor's angle and the determination of the encoder offset. Improved clarity on the relationship between electrical angle and voltage vector.
* Edit encoder offset precise section

* Improve typesetting and clarify vd
…offset-report

Update offset section in the encoder article
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.

6 participants