Ported 'a's to python #224

ArchdukeTim · 2016-12-04T03:26:09Z

No description provided.

virtuald · 2016-12-04T03:48:30Z

wpilib/wpilib/analoggyro.py

-            self.analog.setAccumulatorCenter(self.center)
-            self.analog.resetAccumulator()
+            hal.setAnalogGyroParameters(self._gyroHandle, self.kDefaultVoltsPerDegreePerSecond, offset, center)        
+            reset()


self.reset?

virtuald · 2016-12-04T03:50:38Z

wpilib/wpilib/analoggyro.py

-        if not hal.HALIsSimulation():
-            Timer.delay(1.0)
-
+        if self._gyroHandle == 0:


I don't think this gets initialized before this line?

virtuald · 2016-12-04T03:52:35Z

wpilib/wpilib/analoggyro.py


    def free(self):
        """:see: :meth:`.Gyro.free`"""
        LiveWindow.removeComponent(self)
        if self.analog is not None and self.channelAllocated:
            self.analog.free()
            self.analog = None
-
+        hal.freeAnalogGyro(self._gyroHandle)


Should set handle to None after this

since the code above checks for 0 i'll set to 0

virtuald · 2016-12-04T03:53:00Z

wpilib/wpilib/analoggyro.py

-            Timer.delay(1.0)
-
+        if self._gyroHandle == 0:
+            self._gyroHandle = hal.initializeAnalogGyro(self.analog.port)


By convention, we usually turn m_foo into self.foo

it was a private variable so I added the _ just in case. Should I change it?

I've thought about using that convention, but I think the rest of the code doesn't do that, so let's not start now.

virtuald · 2016-12-04T03:54:09Z

wpilib/wpilib/analogoutput.py

-        except IndexError as e:
-            raise IndexError("Analog output channel %d is already allocated" % channel) from e
-
+        SensorBase.checkANalogOutputChannel(channel)


virtuald · 2016-12-04T03:54:41Z

wpilib/wpilib/analogpotentiometer.py

@@ -49,7 +49,7 @@ def __init__(self, channel, fullRange=1.0, offset=0.0):
        if not hasattr(channel, "getVoltage"):
            channel = AnalogInput(channel)
        self.analog_input = channel
-        self.fullRange = fullRange
+        self._fullRange = fullRange


Why did this get changed?

became private

Yeah, don't change it. Otherwise there's a LOT of things to change..

virtuald · 2016-12-04T05:43:31Z

wpilib/wpilib/analogpotentiometer.py

@@ -60,7 +60,7 @@ def get(self):
        :returns: The current position of the potentiometer.
        :rtype: float
        """
-        return (self.analog_input.getVoltage() / hal.getUserVoltage5V()) * self.fullRange + self.offset
+        return (self.analog_input.getVoltage() / hal.getUserVoltage5V()) * self._fullRange + self._offset


Forgot one.

virtuald

Looks good other than that.

virtuald · 2016-12-04T06:02:24Z

wpilib/wpilib/analoggyro.py

@@ -37,7 +37,7 @@ class AnalogGyro(GyroBase):
    kDefaultVoltsPerDegreePerSecond = 0.007

    PIDSourceType = PIDSource.PIDSourceType
-
+    gyroHandle = 0


Spacing, should have at least one line above each function.

Also... this feels a bit odd to me. It's not wrong. But. I'd rather see this assigned in the constructor.

virtuald · 2016-12-05T06:50:41Z

wpilib/wpilib/analoggyro.py

@@ -56,6 +56,9 @@ def __init__(self, channel, center=None, offset=None):
        :param offset: Preset uncalibrated value to use as the gyro offset
        :type offset: float
        """
+
+        gyroHandle = 0


self.gyroHandle

virtuald · 2016-12-05T06:51:19Z

wpilib/wpilib/analoggyro.py


-
+
+        self.gyroHandle = 0


Why is this here?

couldn't tell you

virtuald · 2016-12-05T06:52:06Z

wpilib/wpilib/analoggyro.py


    def free(self):
        """:see: :meth:`.Gyro.free`"""
        LiveWindow.removeComponent(self)
        if self.analog is not None and self.channelAllocated:
            self.analog.free()
            self.analog = None
-
+        hal.freeAnalogGyro(self.gyroHandle)
+        self.gyroHandle = 0


Since there are changes to be made... spacing!

virtuald · 2016-12-05T06:53:23Z

wpilib/wpilib/analogpotentiometer.py

@@ -60,7 +60,7 @@ def get(self):
        :returns: The current position of the potentiometer.
        :rtype: float
        """
-        return (self.analog_input.getVoltage() / hal.getUserVoltage5V()) * self.fullRange + self.offset
+        return (self.analog_input.getVoltage() / hal.getUserVoltage5V()) * self.fullRange + self._offset


Somehow I missed this (perhaps because of fullrange), but line 53 says that offset doesn't start with an underscore.

removed lock Edited HALReporting and HALUsage fixed typo Fixed typo and moved gyroHandle Fixed changes

virtuald reviewed Dec 4, 2016

View reviewed changes

ArchdukeTim force-pushed the startsWithA branch 2 times, most recently from fb3eb92 to 7213fa4 Compare December 4, 2016 04:39

virtuald reviewed Dec 4, 2016

View reviewed changes

ArchdukeTim force-pushed the startsWithA branch from 7213fa4 to d2ff096 Compare December 5, 2016 06:30

virtuald reviewed Dec 5, 2016

View reviewed changes

Ported 'a's to python

b14bcb5

removed lock Edited HALReporting and HALUsage fixed typo Fixed typo and moved gyroHandle Fixed changes

ArchdukeTim force-pushed the startsWithA branch from d2ff096 to b14bcb5 Compare December 5, 2016 07:24

virtuald merged commit 5f915c0 into robotpy:master Dec 6, 2016

ArchdukeTim deleted the startsWithA branch December 25, 2017 21:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ported 'a's to python #224

Ported 'a's to python #224

ArchdukeTim commented Dec 4, 2016

virtuald Dec 4, 2016

virtuald Dec 4, 2016

virtuald Dec 4, 2016

ArchdukeTim Dec 4, 2016

virtuald Dec 4, 2016

ArchdukeTim Dec 4, 2016

virtuald Dec 4, 2016

ArchdukeTim Dec 4, 2016

virtuald Dec 4, 2016

virtuald Dec 4, 2016

ArchdukeTim Dec 4, 2016

virtuald Dec 4, 2016

virtuald Dec 4, 2016

virtuald left a comment

virtuald Dec 4, 2016

virtuald Dec 5, 2016

virtuald Dec 5, 2016

ArchdukeTim Dec 5, 2016

virtuald Dec 5, 2016

virtuald Dec 5, 2016

Ported 'a's to python #224

Ported 'a's to python #224

Conversation

ArchdukeTim commented Dec 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

virtuald left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment