-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
work around interlock hardware issue #117
Comments
NB AFAICT this is the only "critical" issue currently. The rest is documentation / usability or lower priority enhancements. |
I think adding an additional transform doesn't actually address the problem - there is logically only a single power detector per channel. If we supplied two different transforms, we would then have two means of interpreting the same voltage level. This would essentially just hide the issue, which is what was happening in the previous firmware and why we are just now finding it. I think what adding a second transform would ultimately result in (and what the previous firmware likely does) is that the interlock threshold transform would have an additional 6dBm offset from the power measurement readout. That would have (essentially) the effect of always setting the interlock threshold 6dBm higher than what the power detector actually detects. It's likely easier for us to hard-code that offset value into the firmware and make a note of the hardware fault until we can implement a suitable fix. That being said, ultimately I'd like to hold off on any firmware changes until he problem is more understood. For example:
|
@ryan-summers thanks for the response
This is not the case (see my various measurements on the issue threads). In brief:
|
The issue may well also affect the reflected power interlock. I'll have a look at that at shortly. The threshold for the reflected power is much less critical so it may not actually be a problem (at least for us), otherwise the fix will need applying there as well. |
I fully agree that this issue needs resolving, ideally before any more hardware ships. However, I don't think that waiting until the issue is understood before implementing a workaround is going to work for us.
|
From the data I already have, it does provisionally look like an offset will be enough, but we should take a little more data to confirm that's enough. We also need to verify that the detector -> threshold offset is constant between channels. If both of those work out, then I agree that hard-coding an offset is the most effective resolution here. Assuming this is fixed for the next hw revision we should probably do a runtime check based on the hw-rev pins to determine whether or not to apply the offset, but that can be left for a later stage. @ryan-summers would you mind measuring the detector -> threshold offset (output power only) on a couple of channels of one of your Boosters at a few different powers? I'll do the same and then we can decide what the lowest effort work around is. |
I agree that Cti/ts or you guys at Oxford are best equipped to debug the hardware issue. I don't want to break the seals and don't consider the work around required or critical for the correct operation of the firmware. The easiest work around is to just choose between accurately calibrated monitoring and some level of accurately calibrated thresholds. The latter transformation can just as well be implemented by the user on the host side using the same code they'd develop for calibration. My impression given the am-am keying distortion is that the latter will be ambiguous in any case. |
Hardcoding the heuristic 6 dB offset is also fine as a workaround. It would (and would have been in the specification phase) be very useful to confirm it on some existing devices and it's accuracy over a range of powers. |
That this isn't totally correct AFAICT (but correct me if my reasoning is not right here). There are two voltages: the comparator reference and the detector voltage. The slopes of the DAC and ADC will not be identical. The calibration we currently do includes the ADC reference voltage/ADC gain errors as well as the detector slope, coupler etc. However, the DAC reference/gain errors are independent from the ADC. So, if the ADC reference/gain were far off, the calibration process could actually make the interlock threshold less accurate rather than more. The assumption of course is that the ADC/DAC reference/gain errors are small compared with other errors, so it's fine to just use one calibration for everything. That's probably true, but it's not a priori obvious and needs verification, which I'm not aware of being done. |
I do plan to have another look at that at some point soon. But, to put things in perspective, the level of distortion (presumably thermal transients in the amp) that you saw was only 0.6dB. So worst-case the interlock should still be good to ~1dB, which is fine for all the applications I have in mind. 6dB errors are not. |
Agreed. I think this is will probably be an appropriate work around, but let's confirm first. As I said above, I'm happy to test on a few channels that I have access to right now at a few powers, but it would also be great if @ryan-summers is able to do the same on his hw. |
Sorry, I realize I forgot to mention it here - I don't have a signal generator capable of generating the input RF signals to test this at my office. Previously, Robert was doing additional testing to verify the interlocks, but I believe he no longer has access to a Booster at his office. |
Okay, if you can't easily test then don't worry. If I test a selection of channels that should give us enough information to decide on an acceptable workaround for now. |
AFAWK (#97) relative ADC/DAC gain/offset errors are not a problem and it's not relevant to this problem, right?
You appeared convinced that it would work and the specification/design relies on it. Lesson for next time: If there was doubt, this should have been verified long ago (taking a look at calibrations of existing devices). |
I don't think that contradicts anything I said. I don't expect any issues here and am not advocating changes. |
I'm not sure that's a productive line to go down. Let me have a look at the hw I have here and see how reliable the offset is. If it's something we can hard code into the firmware then the software workaround becomes pretty trivial. I don't want to spend more time discussing this than the fix will take to implement. |
Okay, I have some code written to programmatically scan the synth power in 0.1dB steps and read out the highest value of the output power (as measured by Booster's power detectors) that does not result in an interlock trip. I'm currently blocked by being unable to enable my Booster, but once I get past that I'll run this for interlock thresholds of [13dBm, 23dBm, 33dBm] on a couple of channels and see how things look. |
okay...worked around that by software enabling and re-running the tune script.
So on this channel the issue does seem to induce a fixed 6.2dB discrepancy between the interlock trip and the power readout |
Now looking at another channel in the same Booster. NB I haven't calibrated this channel yet, but I don't expect that to matter since this is a differential measurement (only involving Booster's detector) and, the calibration seems to only make a small difference to the reading any way (at least it did on the other channel I looked at)...
|
Okay, so still a pretty small sample size, but I find that pretty convincing. @ryan-summers are you okay to add a fixed 6.0dB offset in the interlock as a work around for this? |
Ryan's suggestion is a potential workaround if it is supported by analysis and data. |
Yes, this is just on the current v1.5 release. That's the only release I have access to for now so I don't see us realistically being able to test on any other revision any time soon. However, it's highly likely that the behaviour will be the same for v1.4 since there were no relevant changes between releases (basically just the TVS diodes). Revisions before v1.4 are not currently planned to be supported by this firmware anyway (there are quite a few changes to the InAmps etc which would prevent the firmware from working as is). Obviously, more testing is always better, but given the above I'm happy with only testing on v1.5 hardware |
sure, but that's going to take some time and it's not unreasonable to want a work around until then. Why so much opposition to adding an offset into the firmware? It's not a large amount of work to implement and it will make it easier for me to start using the Boosters in the lab and do further testing. If further testing reveals that this offset doesn't work robustly on multiple boards we can always revert the commit. |
I have to disagree. It is a significant amount of work to implement, test, review, understand, verify, document, deploy, revert, maintain, support, revise. We can't stop half-way into the "implement" aspect and ignore the rest. |
@ryan-summers am I right in thinking that replacing: Line 562 in c0a3ccd
power + 6.0 should work as a quick hack?
|
Yes, but that does mean that there will be inconsistencies for example when users query the set interlock threshold. That feels like an unnecessary foot gun that I don't want to have to explain to every student who uses Booster.
I can easily get someone from the Uni to post calibration numbers from the old firmware, but it's such a spaghetti mess that I would have trouble extracting meaningful information from it. Based on past experience, I'm also not overly optimistic about getting wizath et al to help us here. If you're okay to interpret the data then I'm more than happy to gather it. |
hmmm...the change I suggested above doesn't quite achieve the desired effect because the interlock threshold that is reported from the device tries to account for the DAC quantization Line 569 in ec90a61
IMHO we should report the value set by the user, rather than trying to back out the exact setting from the DAC value. The quantization here is much smaller than the other uncertainties (variation over frequency/temperature of the various RF components, offsets in the DAC which we don't calibrate) so accounting for DAC quantization is false precision. I'll remove that in our local checkout next time I reflash. |
I agree that the quantization noise is somewhat unsavory - the original intent here is to show the user the actual setting instead of what was requested. I was originally using this as a means of verifying proper functionality of the firmware. E.g. if I requested a threshold of 10dBm and the firmware then reported something like a 20dBm set threshold, it was obviously not working as intended. We can revert this now since it may no longer be necessary, but we will lose a form of feedback from the firmware for the peace of mind of seeing a nice round number. I'm okay with either direction (the fix is quite small as you note). |
I see, that makes sense. Ideally one should either get a clear error or the interlock threshold one asked for to within the accuracy tolerance. I'm not sure how that works in MQTT, but if I ask for 10dBm and get 20dBm without error then that's a big issue. This is a safety feature primarily so 20dBm could result in catastrophic damage. |
As I mentioned, that was just how I was using it during firmware development. That being said, we could theoretically implement a verification step in firmware to check the quantized setting against the requested to verify they're within a reasonable limit (e.g. 0.5 dBm or so) and then report the user-provided value to gain the same level of certainty. |
Up to you. So long as you're happy things are now working reliably I'm happy dropping this diagnostic |
Current Boosters are being manufactured with this hardware issue addressed in hardware. Older Boosters should (a) be reworked/upgraded, (b) use a recalibrated transform or (c) apply a hack in firmware that offsets the interlock DAC voltage with a guesstimate of the glitches. |
Due to #97 the interlock thresholds are not accurate enough to be useful. It appears highly likely that the underlying issue is a hardware issue. That needs to be understood and resolved, so I've opened sinara-hw/Booster#375
Whatever the underlying cause, there are too many Booster units currently in active use to start applying hardware fixes, so it would be great to find a software workaround for this.
The previous firmware had a separately calibrated transformation for the interlock and power readout. IIRC we decided not to do that in this firmware implementation because it should not be needed (in the sense that the additional accuracy gained by the calibration at one operating point should be small compared with the expected variation over power/frequency/etc -- although I have not thought too carefully about this / tested it on hardware so that may not be totally correct, but I'd be surprised it it were be far off). With separate calibrations, the interlock did work well, suggesting that whatever the cause of this effect, it can be removed by an additional two-point calibration.
@ryan-summers could you look into adding an additional transform that I can set? (or if you have a better idea, let me know!)
The text was updated successfully, but these errors were encountered: