Update transmit and receive firmware to meet timing requirements

[mohamed-dek1]

Closes #104

Notes

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

Self-Review

In this section, please self-review (answer all questions) on a suitable review checklist prior to requesting review from others. Select a review checklist based on what content is being merged in; see the Review Checklists section.

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]

Per 4/6:

With the most recent firmware changes, I've been able to bring the sensor acquisition delay down to about 49us or a max of 20kHz. Since it takes about 6us to send one AMDS worth of data, 2 boards would take about 12us. This is on top of the 11us for a full transmission of one AMDS set of data. Therefore, 3 sets of AMDS data should take approximately 23us. If you add 1-2us for processing incoming data, that would bring us to roughly 27us or a max of 37.04kHz. This means that with a perfect daisy-chain, we still wouldn't be able to meet the timing requirement of 50kHz.

[elsevers]

Thanks @mohamed-dek1. I see your approach here with drv_uart_putc_fast.

One minor point in process_single_byte()

AMDS/Mainboard/Firmware/motherboard_v1/Core/Src/drv_uart.c

Lines 94 to 105 in 0e58045

	case STATE_GOT_HEADER:
	track->data[0] = byte;
	track->state = STATE_GOT_MSB;
	break;
	case STATE_GOT_MSB:
	track->data[1] = byte;
	// --- CUT-THROUGH TRANSMISSION ---
	// The exact microsecond the packet is complete, blast it out
	drv_uart_putc_fast(target_uart, track->data[0]);
	drv_uart_putc_fast(target_uart, track->data[1]);

Move this function call drv_uart_putc_fast(target_uart, track->data[0]); from line 104 to 95 to slightly speed things up

Bigger point: only send sensor card data that the AMDC needs

Yes, you have hit the crux of the issue: if we daisy chain 3x AMDS's deep and send all 8 sensor cards worth of data, we will never hit timing. As soon as we have this implementation going as fast as we can get it, we need to make the AMDS's only transmit data that the AMDC wants them to send: this could be done by:

• detecting what sensor cards are present and only transmitting those cards
• or having the AMDC tell the AMDS which cards to transmit data for (this is the best, of course, but maybe tricky to implement?).

For now, our agreement from our discussion on the whiteboard the other week was to have a variable that in the AMDS code that indicates which sensor cards to try to read/send data on. Later, we can figure out how this variable gets updated. But for now, assume a variable so you can get the logic implemented for how you read ADCs and send their data.

[mohamed-dek1]

Thanks @elsevers, after implementing the selective channel transmit, it's improved quite a bit. Since I have to check whether each channel is active before transmitting, this adds a ~1us delay between when the two UART lines start transmission. When I programmed 3 AMDS boards with a fixed 2-channel transmit mask, the only 2 channels on the FBC, I was able to get the full transmission down to 18.99us. This would meet the timing requirement of 50kHz

Saleae capture

Additional delay

[mohamed-dek1]

Per 4/7 conversation with @elsevers:

• Investigate getting rid of the UART complete wait states?
• Create an additional ADC read function for when only the first set of ADC daisy-chained data is requested

[mohamed-dek1]

I'm running into an issue now where one of the UART lines throws an error and stops sending out any daisy-chained data. After looking into it further with breakpoints, I found that it was a parity error that was causing this issue. Not sure how it's still sending out its own data (the lonely 3 packets).

[mohamed-dek1]

I fixed the issue by clearing the parity error flag when it gets set. Once I start using actual cables instead of jumper cables, we probably won't run into this issue anymore. With the new changes, I've got the transmission down to 17.78us.

Saleae capture

Parity Error

[elsevers]

Thanks @mohamed-dek1. I am trying to understand the delay between when AMDS 1 Data 1 starts transmitting and AMDS 1 Data 0 starts transmitting. Is it because in void EXTI3_IRQHandler(void) we are trying to send three bytes in a row to AMDS 1 Data 1 before attempting to write to AMDS 1 Data 0?

AMDS/Mainboard/Firmware/motherboard_v1/Core/Src/adc.c

Lines 163 to 179 in 824bb43

	for (int i = 0; i < 4; i++) {
	uint8_t header = 0x90 | i;
	// Check Channel 0-3 (UART2)
	if (active_sensor_mask & (1 << i)) {
	drv_uart_putc_fast(USART2, header);
	drv_uart_putc_fast(USART2, (uint8_t)(new_data[i] >> 8));
	drv_uart_putc_fast(USART2, (uint8_t)(new_data[i] & 0xFF));
	}
	// Check Channel 4-7 (UART3)
	if (active_sensor_mask & (1 << (i + 4))) {
	drv_uart_putc_fast(USART3, header);
	drv_uart_putc_fast(USART3, (uint8_t)(new_data[i + 4] >> 8));
	drv_uart_putc_fast(USART3, (uint8_t)(new_data[i + 4] & 0xFF));
	}
	}

It then also seems like there is a long delay until the daisy chained data starts emerging from AMDS 1 Data 1... why? It looks like the data arrives at the AMDS long before it gets sent.

[mohamed-dek1]

Is it because in void EXTI3_IRQHandler(void) we are trying to send three bytes in a row to AMDS 1 Data 1 before attempting to write to AMDS 1 Data 0?

Yes. Since we aren't sending bytes "simultaneously", there's a delay due to the conditionals and also the sequential transmission.

[elsevers]

Is it because in void EXTI3_IRQHandler(void) we are trying to send three bytes in a row to AMDS 1 Data 1 before attempting to write to AMDS 1 Data 0?

Yes. Since we aren't sending bytes "simultaneously", there's a delay due to the conditionals and also the sequential transmission.

Yes, exactly this is THE OPPORTUNITY to fix this! Here is where we can write clever code and compiler optimization to get it tuned up and working right.

[elsevers]

@mohamed-dek1 -- can you please report out the results of our profiling experiments yesterday?

Two things:

1. The baseline + timing improvements we made to EXTI3_IRQHandler(void) relative to the baseline
2. The baseline + timing improvements you were were working on for the entire 3 AMDS daisy chain

[mohamed-dek1]

Below is the most recent capture of an attempt at a full 24 sensor transmission. I'm only seeing the packet drops when at 24 channels, but at 9 channels, it's very consistent. It's currently about 46us like this, but with 9 channels (FBC configuration), it's down to about 14us. I will get more up-to-date captures tomorrow when I have more time to run it again.

[elsevers]

Thanks @mohamed-dek1 -- part of my request here is for you to archive what we wrote down on that sheet of paper in the lab last week so we know it going forward.

[mohamed-dek1]

Below is the notes from our conversation and a screenshot of the baseline timing for the current AMDS changes.

Notes from 4/10

Full transmission capture

[mohamed-dek1]

After switching the process_single_byte(...) function from void to static inline void, I was able to improve the transmission a lot. The issue with the UART lines dropping packets is mostly because of a parity error that keeps happening when the DMA receives data. Below is a capture of a full 36-byte (3 full AMDS) capture, as well as another of the UART line dropping packets.

36byte 35.216us capture

3 bytes dropped on both lines of AMDS 1

Another possible cause of this issue is the DCA board between AMDS 1 and 2 having a hardware issue.

[mohamed-dek1]

I moved the process_routing function to the end of the interrupt, and it's improved a bit.

[mohamed-dek1]

The second transmission, since the first is usually slower.

[elsevers]

Thanks @mohamed-dek1. Do you have any update on stabilizing the communication link (i.e., swapping out your daisy chain adapter boards)?

[mohamed-dek1]

Below is the current timing where each AMDS is only sending 2 channels (FBC style). The transmission time is now about 14us

[mohamed-dek1]

Thanks @mohamed-dek1. Do you have any update on stabilizing the communication link (i.e., swapping out your daisy chain adapter boards)?

I will be doing this today

[mohamed-dek1]

After swapping the DCA boards around, I was getting much better results. I tested 3 transmissions and didn't lose any bytes. Once I switched from a manual trigger to automatic, I began losing packets more frequently, and at some point, the 2 UART lines of AMDS 1 stopped sending daisy chained transmission. I also noticed that AMDS 2 didn't have any issues with it sending it's daisy chained data, which makes me a bit confused.

Image of setup after swapping DCA boards

Capture from MANUAL trigger

Capture from AUTO trigger

[mohamed-dek1]

After removing the state machine, I was getting much faster transmission times. I will need to however, add a count so that I know when a byte is a header vs just data.

[elsevers]

@mohamed-dek1 -- I have spent the morning working on this and have code I would like you to test. Please see my instructions here directing you to merge the code into this branch and explaining the key improvements.