hotfix - line follower kernel issue #273

RobertLucian · 2017-10-09T12:23:10Z

This solves this issue: RobertLucian#8.
The initial report made by John is here: raspberrypi/firmware#828 (comment)

Explanation

Also, the alternative package that was used for interfacing with the line follower sensor is only supported in Python 3, which means that if someone wants to read data from it, the modules have to be imported in Python 3.

At the moment, I haven't removed the support for Python2, so anything that uses the line follower sensor in Python 2 (aka Scratch or GoPiGo3) won't work. Therefore, this PR needs further modifications before it's merged.

The only single way of keeping Python 2 around is to delve into this issue with the I2C and create a library for it in C++. Then some python bindings would be required in order to make the library importable in python and there goes the time. I find this solution to be best of them all, but it takes time and I don't think it's justifiable to do this just so we can keep Python 2 around.

So, I suggest that we move forward with Python 3. In order to do this, we would need to do the following:

Rename this branch to a feature instead of a hotfix and make it PR to the develop branch - the reason is that this is starting to look like a big mod to me and things may go haywire with the master branch.
Remove support for Python 2 and exclusively support Python 3 in this repo.
Remove support for Python 2 and exclusively support Python 3 in the GoPiGo3 repo.
Make DexterOS support Python 3.

Also, to me, I find it peculiar that a sensor which is used on the GoPiGo3 is kept on the GoPiGo repo - I think this has to change as it makes the GoPiGo3 (and thus DexterOS) completely dependent on the GoPiGo repo and this causes us quite some delays because now lots of things have to change - by the way, this is similar to what we have with the distance sensor @CleoQc .

Testing Hotfix

In order to test the hotfix, follow the instructions:

Install the latest kernel for the rpi: sudo rpi-update 5d03351413d4021cc3feeb04459eea98bcb731c0 - this will update to version v.4.9.53-v7.
Git checkout my branch.
Run the install script found in /Setup/ folder.
Start a python 3 shell and enter:

from line_follower import line_sensor
line_sensor.read_sensor()

Also, here's a chart with the tests I have run for the GrovePi and the line follower sensor. It looks like the GrovePi works in all cases, which is great, but the line follower sensor doesn't.

TL;DR

Can't hotfix kernel issue without pushing for Python 3 and "independence" of the software product. But even so, you should test it just as it is right now.

Update

@johnisanerd , @CleoQc the python scripts now use the python-periphery package as a tool for interfacing with the line follower sensor. I have tested it with the latest kernel version and with Python 2 and Python 3 and it works.

Merge Requirements

Add support for installing python packages on DexterOS so we can accept this PR which uses the alternative python-periphery package for interfacing with the line follower sensor.

…tarts

…necessaries

…dequate shebangs

This reverts commit 46be676.

johnisanerd

One suggestion (clone the repo) and one request (documment the issue just a little more in the changed code so that we're kind to our future selves trying to remember what the issue was in the first place).

johnisanerd · 2017-10-12T16:59:16Z

Software/Python/line_follower/line_sensor.py

 import time
 import math
 import RPi.GPIO as GPIO
 import struct
 import operator
 import pickle
 import numpy
+from periphery import I2C, I2CError


Small request here. Can you put a comment explaining why we switched to this, and put a link into this PR since you wrote a detailed explanation about what you found (#273) and maybe also link to the original firmware bug report (raspberrypi/firmware#828) so if we ever have to pick this issue apart again, we have this info a little more readily accessible.

johnisanerd · 2017-10-12T17:00:40Z

Setup/install.sh

+    sudo pip3 install -U RPi.GPIO
+    sudo pip3 install pyusb
+    sudo pip3 install numpy
+    sudo pip3 install python-periphery


An Idea: clone this repo to the DexterInd account. Check on changes once per quarter. This way, changes in the python-periphery library won't suddenly break things for us, forcing us to scramble and fix them.

Just an idea, not a requirement.

We're doing the fixed-version thing instead.

Added requested changes.

johnisanerd

Great work! From my end this is a go. Go!

johnisanerd · 2017-10-13T13:08:50Z

Setup/install.sh

@@ -81,6 +81,12 @@ install_dependencies() {
    sudo pip install -U RPi.GPIO
    sudo pip install pyusb
    sudo pip install numpy
+    sudo pip install python-periphery==1.1.0


johnisanerd · 2017-10-13T13:09:08Z

Software/Python/line_follower/line_sensor.py

+# PR             : https://github.com/DexterInd/GoPiGo/pull/273
+
+
+debug = 0


NICE. Thank you very much @RobertLucian !

CleoQc · 2017-10-17T14:20:58Z

Software/Python/requirements.txt

@@ -1 +1,2 @@
 future
+quick2wire-api


is quick2wire still required?

Nope. Good catch. This is just a left-over. I'm deleting it.

CleoQc · 2017-11-15T13:50:29Z

Ready to merge?

RobertLucian · 2017-11-15T13:51:29Z

I'm testing it as of this moment on DexterOS just to be sure nothing gets messed up. If everything looks good, then I'm going to merge it.

RobertLucian · 2017-11-15T14:13:51Z

@CleoQc @johnisanerd tested it on DexterOS and the line follower sensor works with the new library. I've verified the functionality both in the terminal (by importing the sensor and calling the function) and by going in Bloxter and selecting the line follower sensor.

RobertLucian added 8 commits October 9, 2017 13:28

ref #8 : hotfix - used alternative library that doesn't do repeated s…

1da45f9

…tarts

ref #8 : hotfix - corrected name of dependency

dadd967

ref #8 : hotfix - cured some syntax errors

1214619

ref #8 : hotfix - removing import line that's causing errors + rms un…

303eb02

…necessaries

ref #8 : hotfix - removed python2 support and added python3 + added a…

7c52e18

…dequate shebangs

ref #8 : hotfix - removed support for Python2

46be676

Revert "ref #8 : hotfix - removed support for Python2"

7943480

This reverts commit 46be676.

ref #8 : hotfix - brought back support for python 2 even it won't work

ba37428

RobertLucian added the problems label Oct 9, 2017

RobertLucian self-assigned this Oct 9, 2017

RobertLucian requested review from CleoQc and johnisanerd October 9, 2017 12:23

RobertLucian added 4 commits October 12, 2017 18:24

ref #8 : hotfix - replaced quick2wire with periphery package

0cbb5e0

ref #8 : hotfix - disabled the use of repeated starts

ff629bd

ref #8 : hotfix - solved bug

49aa927

ref #8 : hotfix - further bugs solved

a060cf6

johnisanerd previously requested changes Oct 12, 2017

View reviewed changes

ref #8 : hotfix - added desc of issue + fixed version

53cc756

RobertLucian requested a review from johnisanerd October 12, 2017 18:09

johnisanerd approved these changes Oct 13, 2017

View reviewed changes

CleoQc reviewed Oct 17, 2017

View reviewed changes

CleoQc approved these changes Oct 17, 2017

View reviewed changes

ref #8 : hotfix - quick2wire deleted

1bc7a89

RobertLucian added the pending label Oct 18, 2017

Merge branch 'master' into hotfix/line-follower-kernel-issue

829a6c7

ref #8 : feature - print function error fixed

4e4a392

RobertLucian merged commit 1515fa2 into DexterInd:master Nov 15, 2017

RobertLucian deleted the hotfix/line-follower-kernel-issue branch November 15, 2017 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hotfix - line follower kernel issue #273

hotfix - line follower kernel issue #273

RobertLucian commented Oct 9, 2017 •

edited

Loading

johnisanerd left a comment

johnisanerd Oct 12, 2017

RobertLucian Oct 12, 2017

johnisanerd Oct 12, 2017

RobertLucian Oct 12, 2017

johnisanerd left a comment

johnisanerd Oct 13, 2017

johnisanerd Oct 13, 2017

CleoQc Oct 17, 2017

RobertLucian Oct 17, 2017

CleoQc commented Nov 15, 2017

RobertLucian commented Nov 15, 2017 •

edited

Loading

RobertLucian commented Nov 15, 2017

		# PR : https://github.com/DexterInd/GoPiGo/pull/273


		debug = 0

hotfix - line follower kernel issue #273

hotfix - line follower kernel issue #273

Conversation

RobertLucian commented Oct 9, 2017 • edited Loading

Explanation

Testing Hotfix

TL;DR

Update

Merge Requirements

johnisanerd 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

johnisanerd 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

CleoQc commented Nov 15, 2017

RobertLucian commented Nov 15, 2017 • edited Loading

RobertLucian commented Nov 15, 2017

RobertLucian commented Oct 9, 2017 •

edited

Loading

RobertLucian commented Nov 15, 2017 •

edited

Loading