Add AMDC Firmware changes to implement the DCP

[mohamed-dek1]

Closes #504

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? N/A
5. Is the PR configured to close the correct issue(s)? No
6. Did the PR fully address the Approach section of the issue(s) it is closing? No

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:

• Report checklist
• Publication archive checklist (select a checklist from the Approach that corresponds to the step this PR seeks to complete).
• Standard checklist (if none of the above apply).

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]

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:

• this comment for the hardware test results: Modify the AMDS firmware to receive, handle, and send data from another AMDS as per the DCP spec. AMDS#100 (comment)
• this PR comment for the version of AMDS code: Add AMDS firmware changes to implement the DCP AMDS#102 (comment)

[mohamed-dek1]

I was able to get a full capture that didn't have more than 2.5us delays between bytes. With this transmission, the AMDC was able to read in all the data and display it. Although the ordering is incorrect, the actual data matches what was sent by each FBC. The issue with the ordering is due to us sending the packets out of order and can be fixed by reworking the state machine in the UART drivers.

Full capture

[mohamed-dek1]

@elsevers, I increased the start bit delay from 2.5us to 5us. I've also updated the UART state machine to support out-of-order packets. With these changes, the AMDC is able to receive 3 FBCs worth of data in random order. It validates them properly and then is able to display them to the terminal. At higher switching frequency, I'm getting more and more timed-out bytes, but it's very little compared to the valid packets counter. I tested the current changes by logging a sinusoidal waveform from a function generator. This signal is given to the 3rd FBC, the last in the daisy-chain.

• Signal: 10Hz with an amplitude of 20V
• PWM: 50KHz (what the FBC will be using during current regulation)
• Callback function: 20KHz (control frequency of current-control model)
• Logging: 2KHz

[elsevers]

Per 5/1 meeting:

1. Confirm that packet_counter is no longer needed (and if so, eliminate it)
2. Make AMDS channel memory mapping be continuous for all 24 channels (i.e., AMDS_CH_x_DATA_REG_OFFSET defines should go 0 to 24 without interruption)
3. Update C driver to add funds for indicating which amds channels need to be received for transmission to be complete
4. @mohamed-dek1 has some temporary files in here for testing that will ultimately be removed (i.e., simulink folder)
5. @mohamed-dek1 did I miss anything? Please indicate in the comments

Not discussed in the meeting: other requests from @elsevers:

6. try to make the AMDC firmware such that upgrading doesn't break a user's existing code (so we can think about this being a minor release).
7. eliminate unrelated changes elsewhere (i.e., beta_labs/machine.h should not change)

Since this is not ready for merging, this should be a draft PR. Making that change now.

[mohamed-dek1]

2. Make AMDS channel memory mapping be continuous for all 24 channels (i.e., AMDS_CH_x_DATA_REG_OFFSET defines should go 0 to 24 without interruption)

7. eliminate unrelated changes elsewhere (i.e., beta_labs/machine.h should not change)

These items have been addressed. I've also temporarily increased the bit timeout from 5us to 10, and it's showing much fewer timed-out bytes. The cables have been delivered today so I'll be able to test with proper hardware today if they can get processed in time. I have a bitstream generating currently that removes the packet counter as I've found it to be unnecessary with the new implementation.

AMDC counters running in AUTO

[mohamed-dek1]

1. Confirm that packet_counter is no longer needed (and if so, eliminate it)

addressed

3. Update C driver to add funds for indicating which AMDS channels need to be received for transmission to be complete

@elsevers, I'm not sure what this request means. I don't know where in the firmware it needs to know when a transmission is complete. There are functions for setting and reading the enabled registers' value, which may fulfil this item.

4. @mohamed-dek1 has some temporary files in here for testing that will ultimately be removed (i.e., simulink folder)

This is correct

5. @mohamed-dek1 did I miss anything? Please indicate in the comments

I don't think so.

6. try to make the AMDC firmware such that upgrading doesn't break a user's existing code (so we can think about this being a minor release).

Anyone using an AMDS will currently need to add a line that enables all channels on it. I'm not really sure where to add logic for enabling all channels by default.

[elsevers]

Thanks @mohamed-dek1. Good updates here.

Update C driver to add funds for indicating which AMDS channels need to be received for transmission to be complete

@elsevers, I'm not sure what this request means. I don't know where in the firmware it needs to know when a transmission is complete. There are functions for setting and reading the enabled registers' value, which may fulfil this item.

I had unfortunate wording on this one. This is a simple request to add a wrapper function for setting the verilog register that is used to determine which channels we expect to receive data on (is_dout_enabled?), instead of the user having to directly call the Xil... function.

try to make the AMDC firmware such that upgrading doesn't break a user's existing code (so we can think about this being a minor release).

Anyone using an AMDS will currently need to add a line that enables all channels on it. I'm not really sure where to add logic for enabling all channels by default.

I think we can figure out a way to have a default value indicating we expect to receive data on all channels (is_dout_enabled?). If nothing else, we can add an initialization C code that sets this up to a default value.

[mohamed-dek1]

@elsevers

I had unfortunate wording on this one. This is a simple request to add a wrapper function for setting the verilog register that is used to determine which channels we expect to receive data on (is_dout_enabled?), instead of the user having to directly call the Xil... function.

The user will not have to call the Xil... function. They call the amds_get_data function and then use that to check whatever channel is enabled that they care for.

I think we can figure out a way to have a default value indicating we expect to receive data on all channels (is_dout_enabled?). If nothing else, we can add an initialization C code that sets this up to a default value.

I've added some logic in the amds_init that enables all channels by default.

[elsevers]

@mohamed-dek1

I had unfortunate wording on this one. This is a simple request to add a wrapper function for setting the verilog register that is used to determine which channels we expect to receive data on (is_dout_enabled?), instead of the user having to directly call the Xil... function.

The user will not have to call the Xil... function. They call the amds_get_data function and then use that to check whatever channel is enabled that they care for.

This is not what I mean. The user needs to call a function the sets the bit mapped field is_dout_enabled that the AMDC will use to determine when it has received all of the channels from the AMDS.

[mohamed-dek1]

This is not what I mean. The user needs to call a function the sets the bit mapped field is_dout_enabled that the AMDC will use to determine when it has received all of the channels from the AMDS.

I've added these functions that let you read and set the enabled register. Is this what you're looking for?

uint32_t amds_get_enabled(uint8_t port);
void amds_set_enabled(uint8_t port, amds_channel_e channel);

[mohamed-dek1]

@elsevers, I've update the amds_set_enabled function. This PR is ready for review.

[mohamed-dek1]

I replaced the tabs with spaces, and it's still showing problems. I'm not sure what's wrong.

[AwesomeTornado]

I mistakenly committed my changes directly to this branch instead of making a new branch to merge into this one.
These commits do the same as #510, while preserving hardware logic and version number.

I plan to revert these commits and make a new branch for them so that I may properly test them tomorrow.

I am referencing commits 9ea282d through a8fcd79