Add current sensor calibration documentation #95

anirudhupadhyaya · 2024-04-01T03:52:57Z

I have prepared a first draft of this document. Let me know if this is going in the right direction and your suggestions on what you would like to see.

The document is here.

Thanks.

elsevers

Thanks @anirudhupadhyaya! This is on a good track here and will be really useful. @npetersen2 and I took a look together and have a few revision requests for you to address:

Add a top level discussion (perhaps in Background) on why we need to calibrate a current sensor (i.e., fending off a question of "it has a data sheet, why not just use the values in the datasheet?") and that we need to calibrating against some known reference. In this article you have chosen to use a oscilloscope probe as your reference. But in reality, this is not a great reference. A DMM or precision multimeter would be better (but the scope probe is often convenient and maybe is good enough). The point I am trying to get at here is articulate the concept in general that they need to calibrate against a reference and it is up to the user to pick a suitable reference for their needs
Focus on making this describe the simplest, most general case of current sensor calibration. As part of this, remove the commentary on the AMDS -- this article should apply equally to whether a user is using the AMDS, AMDC analog ports, or some other system.
As part of making this article simpler, could you adjust the perspective of the article to be describing the current measurement calibration for a 3 phase motor? As part of this, please include a figure the shows a three phase motor, current sensor, and drive. And then revise your method to refer to this. (Right now, some of the steps are quite confusing and need clarification to be simpler --> i.e., why does step 4 refer to phase U and step 5 refers to both phase U and phase A?)
Provide a data in CSV file and a Python script, that takes that data and makes the fit (calculates gain and offset) and creates a plot of the raw current measurement data and the resulting fit. Include this plot in your article. NOTE: while of course it is nice if you show us real data, it is also okay if you need to fabricate artificial example data (this is the only time you'll hear me say it is okay to fabricate data!)
Your article correctly indicates that it is best to have both positive and negative currents. Can you revise the data that you are showing to include positive and negative currents?
The current table that you provide shows sensor reading voltages up to 20.0 V. This doesn't seem correct, as the AMDC is not able to read 20 V. Can you provide data the corresponds to the AMDCs capabilities.

anirudhupadhyaya · 2024-04-11T21:00:32Z

@elsevers I have addressed the changes and ready for another review.

...ce/getting-started/control-with-amdc/current-sensor-cal/resources/current_sesnor_drawing.svg

...ting-started/control-with-amdc/current-sensor-cal/resources/Current_sensor_calibration.ipynb

source/getting-started/control-with-amdc/current-sensor-cal/resources/Fit.svg

Minor updates

anirudhupadhyaya · 2024-06-04T04:12:40Z

@npetersen2 I have addressed your review comments..

npetersen2

Thanks @anirudhupadhyaya

Let's get a review from @elsevers now before merging

elsevers

Thanks @anirudhupadhyaya! This is getting close.

I noted a small type-o edit below to make, and then have a request below.

In addition to this, can you please include a [brief] new section called something like "Use of Calibration Data" that gives the user a block of C-Code that uses the measured Offset and Gain to convert between their raw measurements and the values that they should use in their control algorithm?

source/getting-started/control-with-amdc/current-sensor-cal/index.md

anirudhupadhyaya · 2024-06-06T04:38:45Z

Thanks @anirudhupadhyaya! This is getting close.

I noted a small type-o edit below to make, and then have a request below.

In addition to this, can you please include a [brief] new section called something like "Use of Calibration Data" that gives the user a block of C-Code that uses the measured Offset and Gain to convert between their raw measurements and the values that they should use in their control algorithm?

@elsevers I have taken a stab at this. Please take a look.

elsevers

@anirudhupadhyaya I think this is going to be a useful article. Please review/edit/merge my changes in PR #105, and then I am ready to approve.

…sor_cal_doc Edit current sensor calibration document

anirudhupadhyaya · 2024-06-10T03:52:49Z

@anirudhupadhyaya I think this is going to be a useful article. Please review/edit/merge my changes in PR #105, and then I am ready to approve.

@elsevers thanks, I have merged your pull request.

npetersen2 · 2024-06-11T20:54:48Z

source/getting-started/control-with-amdc/current-sensor-cal/index.md

+```C
+#define GAIN 0.621 // Gain from curve fit
+#define OFFSET 4.739 // Offset from curve fit
+
+double current_measurement; // Actual current measurement, to be used in control algorithm
+
+current_measurement = (sensor_reading - OFFSET)/GAIN;  // sensor_reading is the raw measurement and needs to be obtained by the user
+
 ```


Hmm. Is this really how you implement this in your code? I would expect to see a multiply operation used (typically faster), so the inverse operation would be computed at compile time in the define statement, e.g., (1.0 / 0.621) or something like that.

Also, a real implementation would be a bit more "complex" since usually we want to recalibrate during boot-up, so having constants as defines does not work... I do not think you need to give the reader all possible code they would want, but can you add a comment explaining this?

I have added a comment alongside OFFSET define

I suggest changing offset to be a global variable and update the comment. See my suggestion in my review.

source/getting-started/control-with-amdc/current-sensor-cal/index.md

elsevers

Thanks @anirudhupadhyaya. It seems like we are very close here.

source/getting-started/control-with-amdc/current-sensor-cal/index.md

elsevers · 2024-06-25T17:25:21Z

source/getting-started/control-with-amdc/current-sensor-cal/index.md

+```C
+#define GAIN 0.621 // Gain from curve fit
+#define OFFSET 4.739 // Offset from curve fit
+
+double current_measurement; // Actual current measurement, to be used in control algorithm
+
+current_measurement = (sensor_reading - OFFSET)/GAIN;  // sensor_reading is the raw measurement and needs to be obtained by the user
+
 ```


I suggest changing offset to be a global variable and update the comment. See my suggestion in my review.

Co-authored-by: Eric Severson <[email protected]>

anirudhupadhyaya · 2024-06-26T15:04:38Z

@elsevers I have accepted your suggestions.

source/getting-started/control-with-amdc/current-sensor-cal/index.md

…dex.md

elsevers

Looks good.

* Add initial template of new docs control content per #77 * Update index.md * Add current sensor calibration documentation (#95) * Initial commit * Update readme * Update readme * Update readme * Add files * Update files * Changes * Changes * Update files * Get rif of scope image * Minor updates * Address review comments * Remove resources * Add back resources * Address review comments * Address review comments * Update index.md * Edit current sensor calibration method section * Edit calibration method section * Add section on Recalcilating current sensor offset * Clarify that we measure a voltage. * Edit conclusion * Update index.md * Update index.md * Address review comments * Update index.md * Update index.md * Update index.md * Apply suggestions from code review Co-authored-by: Eric Severson <[email protected]> * Update source/getting-started/control-with-amdc/current-sensor-cal/index.md * Update source/getting-started/control-with-amdc/current-sensor-cal/index.md --------- Co-authored-by: Nathan Petersen <[email protected]> Co-authored-by: Eric Severson <[email protected]> * Fix current calibration images (#116) * Add control topic TOC * Remove placeholder articles * Remove unused TOC entries --------- Co-authored-by: Nathan Petersen <[email protected]> Co-authored-by: Anirudh Upadhyaya <[email protected]> Co-authored-by: Nathan Petersen <[email protected]>

anirudhupadhyaya added 3 commits March 31, 2024 19:34

Initial commit

13a70d4

Update readme

389f267

Update readme

9c85d49

anirudhupadhyaya requested review from elsevers and npetersen2 as code owners April 1, 2024 03:52

anirudhupadhyaya changed the title ~~Add cs anirudh~~ Add current sensor calibration documentation Apr 1, 2024

elsevers requested changes Apr 1, 2024

View reviewed changes

anirudhupadhyaya added 6 commits April 10, 2024 23:25

Update readme

f477839

Add files

ba8b2d8

Update files

d070217

Changes

9bdcf98

Changes

11b4a4b

Update files

56110b8

anirudhupadhyaya requested a review from elsevers April 11, 2024 21:00

anirudhupadhyaya and others added 2 commits April 11, 2024 16:01

Get rif of scope image

554b202

Minor updates

5ea2b99

npetersen2 suggested changes Jun 3, 2024

View reviewed changes

anirudhupadhyaya and others added 4 commits June 3, 2024 22:47

Merge pull request #101 from Severson-Group/user/npetersen2/patch-1

996ae82

Minor updates

Address review comments

3371b62

Remove resources

efa5850

Add back resources

c149819

anirudhupadhyaya requested a review from npetersen2 June 4, 2024 04:12

npetersen2 reviewed Jun 5, 2024

View reviewed changes

elsevers requested changes Jun 5, 2024

View reviewed changes

source/getting-started/control-with-amdc/current-sensor-cal/index.md Show resolved Hide resolved

source/getting-started/control-with-amdc/current-sensor-cal/index.md Outdated Show resolved Hide resolved

anirudhupadhyaya added 2 commits June 5, 2024 23:21

Address review comments

fdd3e31

Address review comments

3863bd2

anirudhupadhyaya requested a review from npetersen2 June 6, 2024 04:38

elsevers added 6 commits June 9, 2024 17:28

Edit current sensor calibration method section

911e319

Edit calibration method section

b77e573

Add section on Recalcilating current sensor offset

f48e356

Clarify that we measure a voltage.

f0d00a2

Edit conclusion

cdcb633

Update index.md

3047a09

elsevers requested changes Jun 9, 2024

View reviewed changes

elsevers and others added 2 commits June 9, 2024 18:05

Update index.md

0097469

Merge pull request #105 from Severson-Group/elsevers/edit_current_sen…

fb84857

…sor_cal_doc Edit current sensor calibration document

anirudhupadhyaya requested a review from elsevers June 10, 2024 03:53

npetersen2 suggested changes Jun 11, 2024

View reviewed changes

anirudhupadhyaya and others added 5 commits June 24, 2024 01:09

Address review comments

941ff1b

Update doc

355cf05

Update index.md

4bf6d53

Update index.md

89d7632

Update index.md

e26351e

anirudhupadhyaya requested a review from npetersen2 June 24, 2024 06:26

elsevers requested changes Jun 25, 2024

View reviewed changes

Apply suggestions from code review

5806313

Co-authored-by: Eric Severson <[email protected]>

anirudhupadhyaya requested a review from elsevers June 26, 2024 15:04

elsevers reviewed Jul 1, 2024

View reviewed changes

source/getting-started/control-with-amdc/current-sensor-cal/index.md Outdated Show resolved Hide resolved

elsevers requested changes Jul 1, 2024

View reviewed changes

source/getting-started/control-with-amdc/current-sensor-cal/index.md Outdated Show resolved Hide resolved

elsevers added 2 commits July 1, 2024 11:02

Update source/getting-started/control-with-amdc/current-sensor-cal/in…

b2da7cb

…dex.md

Update source/getting-started/control-with-amdc/current-sensor-cal/in…

93bf498

…dex.md

elsevers approved these changes Jul 1, 2024

View reviewed changes

elsevers merged commit 4024303 into add-control-content Jul 1, 2024

elsevers deleted the add-cs-anirudh branch July 1, 2024 16:03

Add current sensor calibration documentation #95

Add current sensor calibration documentation #95

Uh oh!

Conversation

anirudhupadhyaya commented Apr 1, 2024

Uh oh!

elsevers left a comment • edited by anirudhupadhyaya Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anirudhupadhyaya commented Apr 11, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anirudhupadhyaya commented Jun 4, 2024

Uh oh!

npetersen2 left a comment

Choose a reason for hiding this comment

Uh oh!

elsevers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

anirudhupadhyaya commented Jun 6, 2024

Uh oh!

elsevers left a comment

Choose a reason for hiding this comment

Uh oh!

anirudhupadhyaya commented Jun 10, 2024

Uh oh!

npetersen2 Jun 11, 2024

Choose a reason for hiding this comment

Uh oh!

npetersen2 Jun 11, 2024

Choose a reason for hiding this comment

Uh oh!

anirudhupadhyaya Jun 24, 2024

Choose a reason for hiding this comment

Uh oh!

elsevers Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elsevers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

elsevers Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

anirudhupadhyaya commented Jun 26, 2024

Uh oh!

Uh oh!

Uh oh!

elsevers left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

elsevers left a comment •

edited by anirudhupadhyaya

Loading