Skip to content

Add AMDS firmware changes to implement the DCP#102

Open
mohamed-dek1 wants to merge 85 commits into
developfrom
feature/dcp-amds-firmware
Open

Add AMDS firmware changes to implement the DCP#102
mohamed-dek1 wants to merge 85 commits into
developfrom
feature/dcp-amds-firmware

Conversation

@mohamed-dek1
Copy link
Copy Markdown
Contributor

Closes #95

Notes

Anything reviewers should be aware of when reviewing? Other related issues? Known problems? Future work?

Self-Review

  1. Are all files under 300 kB (if not, please carefully assess whether it is worth committing them)? Yes
  2. Are all files named according to the appropriate naming convention, i.e., dash-case, camelCase, snake case? Yes
  3. Do all Markdown files follow the CONTRIBUTING article template? Yes
  4. Do all links work in the material that the PR is adding? Yes
  5. Is the PR configured to close the correct issue(s)? Yes
  6. Did the PR fully address the Approach section of the issue(s) it is closing? Yes

Reviewer Instructions

Reviewers, please copy and paste a suitable review checklist into your review and answer all questions.

Appendix

This section should be the same for all PRs. Do not edit this section when creating a PR.

Review Checklists

Checklists maintained by the eLev lab for research repositories include:

Standard checklist

1. Are all files under 300 kB (if not, please carefully assess whether it is worth committing them)? **Yes or No**
2. Are all files named according to the appropriate [naming convention](https://github.com/Severson-Group/research-repo-template?tab=readme-ov-file#file-naming), i.e., dash-case, camelCase, snake case? **Yes or No**
3. Do all Markdown files follow the [CONTRIBUTING article template](https://github.com/Severson-Group/.github/blob/main/CONTRIBUTING.md#markdown-documentation-template)? **Yes or No**
4. Do all links work in the material that the PR is adding? **Yes or No**
5. Is the PR configured to close the correct issue(s)? **Yes or No**
6. Did the PR fully address the `Approach` section of the issue(s) it is closing? **Yes or No**

Please work on addressing any **No** items.

@mohamed-dek1 mohamed-dek1 requested a review from elsevers March 23, 2026 15:14
@mohamed-dek1 mohamed-dek1 self-assigned this Mar 23, 2026
@mohamed-dek1
Copy link
Copy Markdown
Contributor Author

mohamed-dek1 commented Mar 23, 2026

We've implemented the Daisy Chain Protocol per this comment. However, we need to ensure that the current implementation meets the timing requirements for 3-phase current control. At this point in time, we are able to exchange data across 2 AMDS boards. The timing spec is not yet met. See

@elsevers
Copy link
Copy Markdown
Contributor

elsevers commented May 13, 2026

Milestone update

Key updates (through PR #122):

At this point, the firmware status is as follows

  • Entire link (AMDC <-- AMDS1 <-- AMDS2 <-- AMDS3) is now at 20 Mpbs (instead of 25 Mbps) because we discovered that the transceivers we are using are only rated to 20 Mbps, and we had started to encounter issues at 25 Mbps
  • Timing: 13.7 us for FBC daisy chain (3x FBCs can reliably send complete data through daisy chain to AMDC from convert start to last byte received). Currently released AMDC/AMDS code does this in ~11 us, see here for baseline data. See Fix timing issue in process routing function #122 (comment) for full details on the FBC timing.
  • Timing: 27 us for AMDS daisy chain (3x AMDSs with 8 sensor cards each can reliably send complete data through daisy chain to AMDC from convert start to last byte received). We had previously calculated a theoretic fastest achievable speed of ~23 us (@mohamed-dek1, can you help me find the comment that documents this fastest speed? I remember typing it up, but can't find it and would like to link it here)
  • The link is working reliably. @mohamed-dek1 is able to log data in a jupyter notebook and it looks correct.

@mohamed-dek1
Copy link
Copy Markdown
Contributor Author

mohamed-dek1 commented May 13, 2026

AMDS testing

AMDS_CH_ENABLE_REG: 0x00111111 Time: ~13.3us

image

AMDS_CH_ENABLE_REG: 0x00F0F0F0 Time: ~30.7us

image

AMDS_CH_ENABLE_REG: 0x000F0F0F Time: ~30.6us

image

AMDS_CH_ENABLE_REG: 0x00FF00FF Time: ~19.6us

image

@mohamed-dek1
Copy link
Copy Markdown
Contributor Author

mohamed-dek1 commented May 13, 2026

I'm able to run a single AMDS at 88KHz on my REVF AMDC with the release candidate. This is what @Daehoon-Sung is running in the cabinet for BP6. For some reason, when I program his AMDS boards with the new firmware, his AMDC is reporting corrupt data. One thing to note is that he has a REVE AMDC which is running (I'm assuming) the most recent released firmware for that board).

88KHz sampling and EVENT_RATIO of 2

image

AMDC counters

image

@mohamed-dek1
Copy link
Copy Markdown
Contributor Author

I've also tested with 2 AMDS boards on 2 different GPIO ports and confirmed still working at 88KHz.

@Daehoon-Sung
Copy link
Copy Markdown

Daehoon-Sung commented May 13, 2026

Hello @mohamed-dek1, for BP4, I am using REVE AMDC and it has the most recent released firmware (1.4.x) for that board. I believe @noguchi-takahiro and I have updated the most recently released firmware. See this page:

image

@mohamed-dek1
Copy link
Copy Markdown
Contributor Author

The timing manager sensor acquisition time versus the Saleae measurement

AMDS_CH_ENABLE_REG: 0x000000F0

image

image

@mohamed-dek1
Copy link
Copy Markdown
Contributor Author

I've programmed @Daehoon-Sung's AMDC with the new firmware and was able to read AMD data through it.

…-rename

Rename project and necessary files from motherboard_v1 to mainboard
@mohamed-dek1 mohamed-dek1 marked this pull request as ready for review May 13, 2026 21:57
elsevers
elsevers previously approved these changes May 13, 2026
Copy link
Copy Markdown
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.

We have thoroughly reviewed and tested this code. Time to squash and merge.

Clarify that this should not be the regular path
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.

3 participants