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.

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

fixed right possibility to 0-100 range instead of 0-99 #206

Closed

Conversation

gamecat69
Copy link
Contributor

@gamecat69 gamecat69 commented Dec 10, 2022

percent() only returns values from 0-0.99 so never actually gets to 100.
In this script, it meant that the right possibility could never get to 100%.
This fix ensures that reading a knob value always returns a 0-100 value.

@mjaskula
Copy link
Collaborator

[discussion] This sounds like a hardware issue.

percent() only returns values from 0-0.99 so never actually gets to 100.

This is not the intention of this method. It should return 0-100. I just ran this program as is (with out this change) in my hardware and I had the full range of percentages available to me on both knobs. (the UI showed 0.00 fully CCW and 1.00 fully CW)

I don't think that this fix is necessary.

@gamecat69
Copy link
Contributor Author

Thanks for reviewing. Your point sent me on a bit of a testing rampage across all my EuroPis!
I found that in some cases neither Bernoulli gate ever gets to 1.00 and in other cases only one gate gets to 1.00.

I then wrote a simple test script (below) and discovered that percent doesn't ever return 1.00, the highest it ever gets to is 99.n.
If others experience the same (more testing required...) do you think we may need something updating in the firmware to round up the return from percent() to 1 if it's above 99.6 say?
Additionally, I think it may be noise that is preventing percent from ever reading 1.00... if you look at _sample_adc() in the Analogue Reader class it produces a mean average of all samples... therefore the value would always be dragged down to a value lower than 1.00 by noise while reading the samples.

    def main(self):
        while True:
            # get pot values
            self.k1percent = round(k1.percent() * 100,1)
            self.k1readposition = k1.read_position(100)

            self.k2percent = round(k2.percent() * 100,1)
            self.k2readposition = k2.read_position(100)

            # display on screen
            oled.fill(0)
            oled.text('k1: ' + str(self.k1percent) + '% | ' + str(self.k1readposition), 0, 0, 1)
            oled.text('k2: ' + str(self.k2percent) + '% | ' + str(self.k2readposition), 0, 12, 1)
            oled.show()

@gamecat69
Copy link
Contributor Author

An updated _sample_sdc() might look something like this:

    def _sample_adc(self, samples=None):
        # Over-samples the ADC and returns the average.
        values = []
        for _ in range(samples or self._samples):
            values.append(self.pin.read_u16())
        val = round(sum(values) / len(values))
        if val < 190:
            return 0
        else:
            return val

Which would be one way to fix the percentage issue across multiple hardware builds. However, it looks like the range() function would never return the highest value because it always deducts 1.... this would need updating as well... unless I am misunderstanding something ...

    def range(self, steps=100, samples=None):
        """Return a value (upper bound excluded) chosen by the current voltage value."""
        if not isinstance(steps, int):
            raise ValueError(f"range expects an int value, got: {steps}")
        percent = self.percent(samples)
        if int(percent) == 1:
            return steps - 1
        return int(percent * steps)

@gamecat69
Copy link
Contributor Author

This is one method (I think) to fix the range function

    def range(self, steps=100, samples=None):
        """Return a value (upper bound excluded) chosen by the current voltage value."""
        if not isinstance(steps, int):
            raise ValueError(f"range expects an int value, got: {steps}")
        percent = self.percent(samples)
        if int(percent) == 1:
            return steps
        return int(percent * steps)

@gamecat69
Copy link
Contributor Author

Suggest this PR is closed in favour of the raised issue: #209

@awonak awonak closed this Dec 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants