quartiq / booster

Firmware for the Sinara Booster RF amplifier
Apache License 2.0
14 stars 1 forks source link

Suggestion: convert configurations from Legacy #155

Open HarryMakes opened 3 years ago

HarryMakes commented 3 years ago

Hi to all, in M-Labs we have come across the lack of compatibility between Legacy firmware and NGFW in terms of configuration data format stored in the EEPROMs. Given a set of data calibrated using Legacy, I'd like to propose a documentation on interpreting the existing data in Legacy format, and convert them to NGFW, possibly done manually or in code with a future PR.

I'm only aware of a similar discussion about documenting NGFW calibration on #107. Please remind me of other threads related to this topic if there's any.

My draft is presented as follows. Please also feel free to point out any mistakes.


Harry's draft ## Conversion Perform conversion to power (in dBm) as required by NGFW. ("EEPROM `@n` \" indicates a value for SYMBOL stored at the byte address n, in decimal; Legacy EEPROM is big-endian) ### Step 1: Output power transform **Target:** \ **Source:** Channel EEPROM `@24` \ Channel EEPROM `@20` \ **Conversion:** 1. Read 16-bit \ as slope `a`, and 16-bit \ as offset `b`. Both are unsigned 16-bit integers. They inversely transform a 16-bit ADC raw sample back to power in dBm: `sample = a * power + b` ==> `power = (sample - b) / a` 2. Given that ADC sets VDDA = 2500 mV (see `main.rs`) and resolution = 12 bits (default value, see `stm32f4xx_hal::adc::config::AdcConfig`, and STM32 manual RM0090, section 13.13.2 about ADC_CR1), find linear transformation from voltage (in V) to power: 1. `voltage = (sample * 2.5) / ((1 \<\< 12) - 1)` (Note: double check with `stm32f4xx_hal::adc::Adc::sample_to_millivolts()` implementation) 2. ==> `power = (voltage * 4095.0 / 2.5 - b) / a = (4095.0 / 2.5 / a) * voltage + (-b / a)` 3. Thus, define LT as: `power = c * voltage + d`, where slope: `c = 4095.0 / 2.5 / a` offset: `d = -b / a` 3. Instantiate LinearTransform object with c and d. 4. Assign the LT object to \. ### Step 2: Output interlock threshold **Target:** \ **Source:** Channel EEPROM `@18` \ Channel EEPROM `@30` \ - see footnote Channel EEPROM `@34` \ - see footnote **Conversion:** 1. Read 16-bit \, which is a 12-bit DAC code. 2. Convert the code to the correct voltage as f32: `voltage = (code >> 4) / (0x1000) * 2.5` (see `Ad5627::set_voltage()`) 3. Convert the \ voltage, using \, to the corresponding power as f32 via linear transformation. Currently, NGFW only uses \ to invert a dBm value for setting the DAC. 4. Assign the power to \. **Footnote:** 1. Legacy uses LT specifically for setting the AD5627 DAC voltage by power, which means this LT is the inverse of NGFW LinearTransform. For this LT on Legacy, 32-bit \ and \ are the slope and offset. Both are float representations in 32-bit, so one must unpack the bytes as f32. 2. Currently, NGFW uses only \ to transform the voltage to power when assigning the threshold. Since this LT is vastly different from the LT used by Legacy for accepting user's input, for the same DAC code, the power value stored on NGFW is not necessarily equal to the power accepted by Legacy from the user, and the two shall have no obvious correlation. 3. As a result, usage of the values \ and \ on Legacy still remains undiscovered for NGFW. ### Step 3: Bias voltage **Target:** \ **Source:** Channel EEPROM `@28` \ **Conversion:** 1. Read 16-bit \, which is a 12-bit DAC code. 2. Convert the code to the equivalent voltage as f32: `voltage = code / 4096.0 * 3.3` (assume `Dac7571::default()`; see `Dac7571::set_voltage()`) 3. Further convert the voltage to one accepted by `rf_channel::set_bias()`: `voltage := -1.0 * voltage` (see comment on `rf_channel::set_bias()`) 4. Assign the final voltage value to \. **Footnote:** 1. Legacy only accepts a DAC7571 DAC code as user's input for setting and storing the bias value, so no conversion is involved. However, NGFW fetches and stores a negated voltage value, and converts it to the bias value with `rf_channel::set_bias()`. ### Step 4: Channel state (optional) *(The Legacy firmware doesn't provide user-facing commands for reading main EEPROM via VCP interface)* **Target:** \ **Source:** Board EEPROM `@30` \ **Conversion:** 1. Read 8-bit \, which is a bitmask where LSB is channel 0. 2. Extract the correpsonding bit for the channel. 3. Assign the bit as bool to \. ### Step 5: Input power transform **Target:** \ **Source:** Channel EEPROM `@38` \ Channel EEPROM `@42` \ **Conversion:** 1. Read 32-bit \ as slope, and 32-bit \ as offset. Both are float representations in 32-bit, so unpack the bytes as f32. They transform a 12-bit ADC raw sample to power in dBm: `power = a * sample + b` 2. Find linear transformation from voltage to power: 1. `voltage = code / 4096.0 * 3.3` (assume `Mcp3221::default()`; see `Mcp3221::get_voltage()`) 2. ==> `power = a * (voltage * 4096.0 / 3.3) + b` 3. Thus, define LT as: power = c * voltage + d, where slope: `c = a * 4096.0 / 3.3` offset: `d = b` 3. Instantiate LinearTransform object with the slope and offset. 4. Assign the LT object to \. ### Step 6: Reflected power transform **Target:** \ **Source:** Channel EEPROM `@26` \ Channel EEPROM `@22` \ **Conversion:** 1. Read 16-bit \ as slope, and 16-bit \ as offset. Both are unsigned 16-bit integers. They transform 16-bit ADC code to power. 2. Find linear transformation from voltage to power, using conversion step 2. for \. 3. Instantiate LinearTransform object with the new slope and offset. 4. Assign the LT object to \.
jordens commented 3 years ago

It's important to first determine which calibrations are actually required and meaningful in practice. The fact that something can be calibrated (deviate from the datasheet and nominal values) doesn't mean that it should be calibrated. The calibration may actually be less accurate than the on-paper value and on top of that VSWR or things like https://github.com/sinara-hw/Booster/issues/375 may make the calibrated values less meaningful than the default ones in several cases.

This was and is unclear and it is also unclear to what extent data from the old firmware actually provides a benefit for the new one. My gut feeling is that state, bias and interlock threshold are the only useful and meaningful ones.

Other than that, sure, on-the-fly conversion of old data would be really nice where this is meaningful and once tested thoroughly to be working robustly.

Without having checked anything, the 4095.0 in your notes should probably all be 4096.0.

HarryMakes commented 3 years ago

@jordens Thanks for the comments. I know the answer to your concern about 4095.0. Actually, it's used for STM32F4's ADC conversion only and matches both the Rust HAL and STM32 official HAL implementations:

stm32-rs/stm32f4xx-hal: ADC digital full-scale value is (1 << bit_resolution) - 1. https://github.com/stm32-rs/stm32f4xx-hal/blob/v0.8.3/src/adc.rs#L761

STMicroelectronics/STM32CubeF4: ADC digital full-scale value is 0xFFF >> ( RES >> (24-1) ), where RES is the resolution setting value (bits 25:24) of register ADC_CR1. https://github.com/STMicroelectronics/STM32CubeF4/blob/4aba24d78fef03d797a82b258f37dbc84728bbb5/Drivers/STM32F4xx_HAL_Driver/Inc/stm32f4xx_ll_adc.h#L1603-L1604 Here's a table for better understanding:

Value of RES[1:0], i.e. ADC_CR1[25:24] ADC_CR1[25:24] >> 23, i.e. RES[1:0] << 1 Equivalent bit resolution
0b00 0b000 == 0 12 - 0 = 12
0b01 0b010 == 2 12 - 2 = 10
0b10 0b100 == 4 12 - 4 = 8
0b11 0b110 == 6 12 - 6 = 6

So apparently, STM32 ADC works differently from ADC ICs (DAC7571, MCP3221). Digging deeper, I found a difference in terms of "last code transition" between STM32 ADC and MCP3221:

STM32 ADC (manual):

3.1.2 Gain error ... The last actual transition is the transition from 0xFFE to 0xFFF. Ideally, there should be a transition from 0xFFE to 0xFFF when the analog input is equal to VREF+ – 0.5 LSB.

MCP3221 (datasheet):

4.8 Gain Error ... The ideal location of the last code transition is 1.5 LSBs below full-scale or VDD.

I can't find similar info for DAC7571, but this StackExchange comment also suggests that the max binary code normally can't differentiate Vref from Vref - 1.5 LSB. So I think STM32 ADC is a special case.

jordens commented 3 years ago

No. Also for the STM32F4 the scale is one LSB per VREF/4096. See e.g. page 10 of the manual you quote. If someone (HALs) divides by 4095, it's simply wrong. Filing an issue would be good.

HarryMakes commented 3 years ago

@jordens Thank you for the advice! I've opened an issue here: https://github.com/stm32-rs/stm32f4xx-hal/issues/362