Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Hardware Issue] Noise on knob values and analogue inputs #209

Closed
gamecat69 opened this issue Dec 22, 2022 · 24 comments
Closed

[Hardware Issue] Noise on knob values and analogue inputs #209

gamecat69 opened this issue Dec 22, 2022 · 24 comments
Labels
hardware - EuroPi Hardware related issue

Comments

@gamecat69
Copy link
Contributor

Hardware Issue

Describe the bug
There is noise present when sampling the analogue inputs or knob values. This noise offsets the value read at the extremities of the readings, e.g 0 will never read 0 and the max value will never read the max value.

This issue causes the the percent() value to never read 100% on some modules. The issue also causes the analogue input to never read 0 Volts.

To Reproduce
Steps to reproduce the behavior:
Read any value from k1 or ain to more than 1 decimal place several times.

Expected behavior
It would be great if something like a deadzone was implemented. e.g. if really close to 0 the value returned is 0 or of close to the max the value returned is the max value.
@gamecat69 gamecat69 added the hardware - EuroPi Hardware related issue label Dec 22, 2022
@redoxcode
Copy link
Contributor

I propose to implement a deadzone percent() function of the Knob class like this:
https://github.com/redoxcode/EuroPi/blob/add-dead-zone-to-knobs/software/firmware/europi.py
But I haven't tested this on hardware and obviously the tests fail, since they don't assume any deadzones

@gamecat69
Copy link
Contributor Author

Note that a deadzone would be needed for both knobs and the analogue input. As the analogue input never truly reads the min or max value (in the same way as the knobs), due to the averaging done in _sample_adc().

@redoxcode
Copy link
Contributor

redoxcode commented Dec 22, 2022 •
edited

I think the analogue input should be able to return values even larger than 1.0 if calibrated because of this code:

    def percent(self, samples=None):
        """Current voltage as a relative percentage of the component's range."""
        # Determine the percent value from the max calibration value.
        reading = self._sample_adc(samples)
        max_value = max(reading, INPUT_CALIBRATION_VALUES[-1])
        return reading / max_value

since you divide by 'max_value', that will be larger than 'reading' if calibrated, it should return 1.0 without reaching 'MAX_UINT16'

@gamecat69
Copy link
Contributor Author

gamecat69 commented Dec 23, 2022 •
edited by awonak

An alternative method would be to mask this problem at a lower level in the _sample_adc function using something like the example below. One benefit of doing this is that it would mask the problem in all higher level functions for both the knobs and the analogue input.

    def _sample_adc(self, samples=None, low_dead_zone=190, high_dead_zone=65500):
        # Over-samples the ADC and returns the average.
        values = []
        for _ in range(samples or self._samples):
            values.append(self.pin.read_u16())
        #return round(sum(values) / len(values))
        val = round(sum(values) / len(values))
        if val < low_dead_zone: # This is just noise, remove it by setting to 0
            return 0
        elif val > high_dead_zone: # probably noise, set to highest value
            return 65535
        else:
            return val

@redoxcode
Copy link
Contributor

I just updated my suggested code. The deadzones are now again in the percent() function of the AnalogueReader class and could be used by knobs and the analog input. But I am still not convinced you get that problem with the analog input in a calibrated module. At least not for 1.0. There might be an issue for 0.0, but it would make more sense to rework how the calibration works to fix this. Using a deadzone with the analog input would likely any attempt to quantize input to V/O harder.
I also made some changes to _sample_adc, but they should only boost the performance and not change the outcome.

@gamecat69
Copy link
Contributor Author

gamecat69 commented Dec 23, 2022 •
edited

I was a little concerned that in our haste to develop a fix, we perhaps had not correctly defined the problem, how to test for it and how to test that any change had fixed it lol! :)

... so I created a simple test script that displays the following:

  1. Noise at the lowest and highest levels when reading from k1 and k2
  2. Noise at the lowest level when reading from ain (I excluded the high val test for ain partially for speed and partially because I do not believe this will cause much of a problem for most)

test code: https://github.com/gamecat69/EuroPiApps/blob/pot-test/software/contrib/noise_test.py

When running the test, I believe the problem is present if any low value is greater than 0 and any high value is less than 1.0.

I ran this test twice, once using the suggested fix to _sample_adc and once using the suggested by @redoxcode.

The _sample_adc update fixed the problem for both the knobs and ain.
The suggestion by @redoxcode fixed the problem for the knobs, but not for the ain.

@redoxcode
Copy link
Contributor

I don't think this should be a implemented in _sample_adc as it returns an in from 0 to MAX_UINT16 that is the direct average of the samples that are in this range them self. Especially with sample size 1 this would result in a strange mapping. _sample_adc should do what everyone expects it to do.
I haven't enabled deadzones for ain in my code since I don't think deadzones are the right way to handle this issue for ain. A change in way the calibration works would make more sense (not just calibrate the high point, but also the 0 point including any noise).
But since my suggestion is implemented in the AnalogReader class it could be enabled for ain as well. But it shouldn't!
I finished my tests on actual hardware and will make a PR later.

@gamecat69
Copy link
Contributor Author

gamecat69 commented Dec 25, 2022 via email •
edited

@redoxcode
Copy link
Contributor

Yes, it could be included in the calibration how large deadzones should be for the calibrated unit. But I don't think it's necessary. It will be hardly noticeable if you need to turn the knob 0.1° further on one unit than the other to get the same result. However for ain it can make a great difference if you have any script that needs precision, since it needs to work with the voltages provided by other modules.

This was referenced Dec 26, 2022
Closed
Merged
@gamecat69 gamecat69 mentioned this issue Dec 27, 2022
Closed
@redoxcode
Copy link
Contributor

Closed?

@gamecat69
Copy link
Contributor Author

gamecat69 commented Mar 27, 2023 via email

@redoxcode
Copy link
Contributor

There was a PR that fixed 2 point calibration (#232)
idk, I don't have that issue so I can't test.

@mjaskula mjaskula mentioned this issue Mar 30, 2023
Closed
@gamecat69
Copy link
Contributor Author

@redoxcode: I am interested to hear what results you get when you run the test script I provided above.
Do you not experience the noise on ain? i.e. is the value 0 when nothing is plugged in when you run the test script?

@redoxcode
Copy link
Contributor

redoxcode commented Apr 5, 2023 •
edited

@gamecat69 I just made a fresh setup with the firmware 0.8.1 (not calibrated) and your script and I get 0.0 and 1.0 for both knobs and 0.0 for ain. I tested with and without usb power (always with eurorack power). Also I tested to have something constant connected to ain and that shows up as some value > 0.0 as expected.

@gamecat69
Copy link
Contributor Author

Thanks for performing the test @redoxcode. I just did the same test on two modules using two different power supplies.
I got 0.0 and 1.0 values for k1 and k2, but non values > 0.0 for the ain test with nothing connected. I wonder where the ain noise is coming from in my setup....

@redoxcode
Copy link
Contributor

redoxcode commented Apr 5, 2023 •
edited

@gamecat69 can you try to do the 2 point calibration and see if that helps?
What happens when you connect a cable with 0V on it?
Also maybe reflow the solder on the 3 pins of the socket?
Is the connection to ground working when nothing is connected?

There was a PR that fixed 2 point calibration (#232)

@redoxcode
Copy link
Contributor

Closed? As OP hasn't answered and #211 (comment) is closed as well?

@gamecat69
Copy link
Contributor Author

gamecat69 commented Aug 15, 2023 via email

@gamecat69
Copy link
Contributor Author

gamecat69 commented Aug 15, 2023 via email

@roryjamesallen
Copy link
Collaborator

Out of interest, what happens if you use the same precision for the calibration? i.e. change calibrate.py to use 4096 samples rather than 256 for each analogue input read. It is good that we're so so close to 0V, and almost any use of ain will naturally round that down to 0 (6 decimal places would let ain select between far more values than the specification of the EuroPi states based on the bit depth of the ADC), so this small discrepancy could just be down to using a lower number of samples when determining the zero point

@roryjamesallen
Copy link
Collaborator

a very small amount of noise on the ain (0.000562).

I've just run your noise script on an uncalibrated europi and got exactly 0.0 on ain, haven't yet tried with a calibrated one

@gamecat69
Copy link
Contributor Author

gamecat69 commented Aug 22, 2023 via email

@roryjamesallen
Copy link
Collaborator

roryjamesallen commented Aug 22, 2023 •
edited

I get higher noise (0.03nnn). If I calibrate using 256 samples I get lower noise (0.00002n).

That is interesting, it does lean towards some kind of spiked noise then rather than a consistent noise floor, and that would explain more samples having higher noise as it picks up more of the spikes.

Either way, I think that this level of noise is acceptable as noise will be different in every system like you say, so any solution would be imperfect: if we find the average user noise at 256 samples and introduce firmware rounding to make sure that's zero, users with noisy cases will still experience noise at zero, and users with clean cases will have every ain reading slowed down by rounding unnecessarily

@roryjamesallen
Copy link
Collaborator

With no realistic solution possible for this issue which doesn't sacrifice the experience of users with lower noise cases, and with the EuroPi-X development in progress which will fix these noise issues using higher quality ADCs and filtering, I'm going to mark this one closed unless any significant findings or solutions are found.
The Pico ADCs are very noisy, and from what I've read this in large part due to limitations of the ADC itself. Maybe in future this can be "fixed" with config menu settings to raise the voltage level that's considered as zero to account for a noise floor, but it might just be something we have to live with and accept that the hardware of the EuroPi is imperfect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
hardware - EuroPi Hardware related issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gamecat69 @roryjamesallen @redoxcode