Manual Merge of #392 into Staging Branch

[codecubepi]

This PR was created to merge the changes authored by @annikaolson into the v1.3-staging branch. I have also made a few additional changes which are noted at the very end of this comment.

Changes authored by Annika in #392

It was necessary to manually merge these changes, as we both had made changes to the FPGA, and the amdc_revf.bd block design file could not be automatically merged with my changes from #394 and these changes. To perform the manual merge, I looked through all the changed files in #392 and, after approval from @npetersen2 , I created a single commit (bd8ae30) for Annika's changes. The following description is copied from #392 and describes the changes made:

This PR addresses issues #391, #314, and #377. Previously, the scheduler ISR was triggered by the SysTick private timer in timer.c. This was initialized to a predetermined control rate, with the default at 10kHz - in other words, the scheduler would fire every 100 μs. The goal here was to instead synchronize the scheduler with the PWM carrier.

Two Scheduler Modes

There are now two modes for the scheduler source/functionality. This is user-configurable in usr/user_config.h by USER_CONFIG_ISR_SOURCE.

Mode 0

The scheduler runs the tasks (i.e. ISR is called to move out of the idle state) on every "trigger" assertion.

• Note that this is not actually based on the trigger signal. The trigger can only be asserted once all the sensors are done (see FPGA Changes - Sensors), but this mode is not synchronized to the sensors and works regardless - it is just synchronized to the PWM! Therefore, the interrupt is actually set when the number of qualified events hits the defined user ratio - this is the same condition as the trigger, except without all_done. The default (in timing_manager.h) is 10, which triggers the ISR every 10 carrier lows, leading to a default control rate of 10kHz.

The user can change the PWM event qualifier (carrier high, low, or both) and the user ratio to change the control frequency of the tasks.

Mode 1

When none of the sensors are enabled, this mode operates the same as mode 0. When any sensors are enabled, the scheduler is synchronized with the sensors. The ISR is called when all of the sensors have completed their acquisition cycles (positive edge of all_done).

FPGA Changes

Sensors

• As per #381, the trigger signal is now based on all_done. For this fix to work, all of the sensors needed to start their done signal at 1

  Timing Manager

• The previous interrupt was asserted upon a positive edge of the all_done signal, which would call an ISR in the timing manager driver to update the stats once all the sensors had completed their acquisition. Now, the interrupt is asserted based on the modes mentioned above:

https://github.com/Severson-Group/AMDC-Firmware/blob/47d34ecf31241087504ffe84d59e5e3b1ca63290/ip_repo/amdc_timing_manager_1.0/src/timing_manager.v#L207-L223

• Now that the interrupt for the scheduler comes from the FPGA, we also needed a way to get the elapsed time between interrupt assertions (or, from the driver side, between ISR calls/scheduler ticks) as it is no longer fixed. A counter was also added that resets upon every interrupt and is copied over to a register that can be read from the C code.

C-Code Changes

cmd_hw.c

To report the time, the code before was directly calling timing_manager_get_time_per_sensor(), but it is now using the statistics struct for each sensor. It's pulling the value (which is the most recent value), but the stats also hold the max, min, and mean value for each sensor time.

timing manager changes

• The ISR that drives the scheduler is now in the timing manager, and does 3 things: update the stats for each sensor, 'tick' the scheduler, and clear the ISR. There is also now a function to set the mode, or the source of the interrupt for the scheduler - this is sent to the FPGA and is used to set the interrupt.
• There's now a function to get the elapsed time between ISR calls that reads from slv_reg4 in the FPGA.

scheduler changes

• Now that the SysTick timer is not being used, timer.c and timer.h could be removed. The timing manager ISR is the only one used, so the code in scheduler_timer_isr() was moved to scheduler_tick() which increments the elapsed time and moves it out of the idle state, unpausing scheduler_run() and runs the tasks.
• All the references to the SysTick frequency also needed to be removed. Most of the tasks had fixed frequencies, but the log task was using the defined SysTick frequency from the scheduler. This means that it should be happening every time the ISR is called, or as often as possible, so the interval is set to 0. https://github.com/Severson-Group/AMDC-Firmware/blob/47d34ecf31241087504ffe84d59e5e3b1ca63290/sdk/app_cpu1/common/sys/scheduler.c#L190-L192 Thus, the elapsed time since the last run will always be greater than 0, and this task will run every time as desired.
• With the SysTick timer not being used, this condition could no longer be used: https://github.com/Severson-Group/AMDC-Firmware/blob/153dd3f76416b4752a0acfcac4cb20288b142683/sdk/app_cpu1/common/sys/scheduler.c#L38 So instead, since startup was not an issue after removing the condition, it was changed to this: https://github.com/Severson-Group/AMDC-Firmware/blob/47d34ecf31241087504ffe84d59e5e3b1ca63290/sdk/app_cpu1/common/sys/scheduler.c#L38 This is because when a sensor is first enabled, the time between ISR calls is very small (since it is measured from the interrupt, where a trigger starts a cycle later, until the sensor is completed, which is much faster than the typical control rate). After this, it has to wait for the trigger again, and it re-aligns with the regular frequency. Since the sensor is so fast, the ISR is called before the previous cycle is done running all the tasks, so this condition ensures that in this scenario, the time quantum checking is ignored.

Block Design Changes

• Removed hier_timers IP. This hierarchy contains two control timer modules that are connected to IRQ_F2P of the processing system - they do nothing, and aren't needed!
• Now, there is an interrupt coming from timing_manage labeled sched_isr - this is connected to In0 of a 16-bit concat block. In1 is a 15-bit port connected to a const block with value of 0. This concat block output should be sent to the IRQ_F2P port of the processing system. The LSB (IRQ_F2P[0]) is the interrupt with ID 61.

Additional Changes

• I decreased the length of the very long defines for the timing manager IP addresses
• I added a section for the new AMDS driver in the ip_repo/README.md, which I had forgotten to do before
• I updated the REV E block design for Anirudh. See this issue comment.

[npetersen2]

Nice work, @codecubepi

I reviewed the last 3 commits on this branch, as in, the changes not co-authored by @annikaolson. those look good to me.

I presume you squashed @annikaolson's previous PR into the single commit here, so since I already reviewed that over there, that should be good.

So.... I approve! :) Now, we just need to confirm this code actually works on the hardware

[codecubepi]

There is one build issue that we should resolve before merging this:

https://github.com/Severson-Group/AMDC-Firmware/blob/8f60d5c1bf2f9b7deea53740cb298e157c3190de/sdk/app_cpu1/common/sys/cmd/cmd_log.c#L64-L68

This fails to build since LOG_UPDATES_PER_SEC was deleted... how should I resolve?

[npetersen2]

Good catch.

In the lastest code, the log task update rate is not constant... it floats, depending on the PWM settings.

So, I think this would change to:

double log_update_rate = 1.0 / (timing_manager_get_tick_delta() * 1e-6);
 if (samples_per_sec > log_update_rate || samples_per_sec <= 0) { 
     // ERROR 
     return CMD_INVALID_ARGUMENTS; 
 } 

or, something like this

[codecubepi]

@npetersen2

The problematic define was also present in log.c so I just updated log.h with a new define that is then available in both log.c and cmd_log.c