Linter python (commaai#299)

* Added pyflakes and Pylint for python

* Actually run linter in CI

* some simple pyflakes changes

* Add flake8 to panda reqs for linter test

* flake8 errors are fixed

* run flake8 in regression tests

* meant to run flake8

* hack to ignore unused import

* bug

* fix bugs in tucan_loopback

* Another fix by using set_safety_mode

* added pylintrc file

* more fixes and enabled pylint as well

* Fix pylint in circleci

* added linter to readme

.circleci/config.yml

@@ -92,6 +92,20 @@ jobs:
           command: |
             docker run language_check /bin/bash -c "cd /panda/tests/language; ./test_language.py"
 
+  linter_python:
+    machine:
+      docker_layer_caching: true
+    steps:
+      - checkout
+      - run:
+          name: Build image
+          command: "docker build -t linter_python -f tests/linter_python/Dockerfile ."
+      - run:
+          name: Run linter python test
+          command: |
+            docker run linter_python /bin/bash -c "cd /panda/tests/linter_python; PYTHONPATH=/ ./flake8_panda.sh"
+            docker run linter_python /bin/bash -c "cd /panda/tests/linter_python; PYTHONPATH=/ ./pylint_panda.sh"
+
 workflows:
   version: 2
   main:
@@ -101,3 +115,4 @@ workflows:
       - build
       - safety_replay
       - language_check
+      - linter_python

README.md

@@ -108,6 +108,8 @@ to ensure that the behavior remains unchanged.
     * compiling the code in various configuration and flashing it both through USB and WiFi.
     * Receiving, sending and forwarding CAN messages on all buses, over USB and WiFi.
 
+In addition, we run [Pylint](https://www.pylint.org/) and [Flake8](https://github.com/PyCQA/flake8) linters on all python files within the panda repo.
+
 Hardware
 ------
 

__init__.py

@@ -1 +1 @@
-from .python import Panda, PandaWifiStreaming, PandaDFU, ESPROM, CesantaFlasher, flash_release, BASEDIR, ensure_st_up_to_date, build_st, PandaSerial
+from .python import Panda, PandaWifiStreaming, PandaDFU, ESPROM, CesantaFlasher, flash_release, BASEDIR, ensure_st_up_to_date, build_st, PandaSerial  # noqa: F401

board/tools/enter_download_mode.py

@@ -11,7 +11,7 @@ def enter_download_mode(device):
 
   try:
     handle.controlWrite(usb1.TYPE_VENDOR | usb1.RECIPIENT_DEVICE, 0xd1, 0, 0, b'')
-  except (usb1.USBErrorIO, usb1.USBErrorPipe) as e:
+  except (usb1.USBErrorIO, usb1.USBErrorPipe):
     print("Device download mode enabled.")
     time.sleep(1)
   else:

crypto/getcertheader.py

@@ -1,6 +1,5 @@
 #!/usr/bin/env python3
 import sys
-import struct
 from Crypto.PublicKey import RSA
 
 def egcd(a, b):

examples/can_bit_transition.py

@@ -1,6 +1,4 @@
 #!/usr/bin/env python3
-
-import binascii
 import csv
 import sys
 

examples/can_unique.py

@@ -13,10 +13,8 @@
 # 0,344,c000c00000000000
 
 
-import binascii
 import csv
 import sys
-from panda import Panda
 
 class Message():
   """Details about a specific message ID."""

examples/get_panda_password.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python3
+import sys
 from panda import Panda
 
 def get_panda_password():

examples/query_vin_and_stats.py

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 import time
 import struct
-import binascii
 from panda import Panda
 from hexdump import hexdump
 from panda.python.isotp import isotp_send, isotp_recv

python/__init__.py

@@ -10,10 +10,10 @@
 import traceback
 import subprocess
 from .dfu import PandaDFU
-from .esptool import ESPROM, CesantaFlasher
-from .flash_release import flash_release
-from .update import ensure_st_up_to_date
-from .serial import PandaSerial
+from .esptool import ESPROM, CesantaFlasher  # noqa: F401
+from .flash_release import flash_release  # noqa: F401
+from .update import ensure_st_up_to_date  # noqa: F401
+from .serial import PandaSerial  # noqa: F401
 from .isotp import isotp_send, isotp_recv
 
 __version__ = '0.0.9'
@@ -27,10 +27,10 @@ def build_st(target, mkfile="Makefile"):
   from panda import BASEDIR
   cmd = 'cd %s && make -f %s clean && make -f %s %s >/dev/null' % (os.path.join(BASEDIR, "board"), mkfile, mkfile, target)
   try:
-    output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, shell=True)
-  except subprocess.CalledProcessError as exception:
-    output = exception.output
-    returncode = exception.returncode
+    _ = subprocess.check_output(cmd, stderr=subprocess.STDOUT, shell=True)
+  except subprocess.CalledProcessError:
+    #output = exception.output
+    #returncode = exception.returncode
     raise
 
 def parse_can_buffer(dat):
@@ -558,7 +558,7 @@ def kline_drain(self, bus=2):
   def kline_ll_recv(self, cnt, bus=2):
     echo = bytearray()
     while len(echo) != cnt:
-      ret = str(self._handle.controlRead(Panda.REQUEST_OUT, 0xe0, bus, 0, cnt-len(echo)))
+      ret = self._handle.controlRead(Panda.REQUEST_OUT, 0xe0, bus, 0, cnt-len(echo))
       if DEBUG and len(ret) > 0:
         print("kline recv: " + binascii.hexlify(ret))
       echo += ret

python/dfu.py

@@ -1,8 +1,6 @@
-
 import os
 import usb1
 import struct
-import time
 import binascii
 
 # *** DFU mode ***
@@ -116,6 +114,6 @@ def reset(self):
     self.status()
     try:
       self._handle.controlWrite(0x21, DFU_DNLOAD, 2, 0, b"")
-      stat = str(self._handle.controlRead(0x21, DFU_GETSTATUS, 0, 0, 6))
+      _ = str(self._handle.controlRead(0x21, DFU_GETSTATUS, 0, 0, 6))
     except Exception:
       pass

python/esptool.py

@@ -29,7 +29,7 @@
 import sys
 import tempfile
 import time
-import traceback
+#import traceback
 import usb1
 
 __version__ = "1.2"
@@ -44,7 +44,7 @@ def __init__(self, serial=None):
 
   @property
   def baudrate(self):
-    return self._baudrate
+    return self.baudrate
 
   @baudrate.setter
   def baudrate(self, x):
@@ -434,7 +434,7 @@ def append_checksum(self, f, checksum):
 
     def write_v1_header(self, f, segments):
         f.write(struct.pack('<BBBBI', ESPROM.ESP_IMAGE_MAGIC, len(segments),
-                            self.flash_mode, self.flash_size_freq, self.entrypoint))
+                            self.flash_mode, self.flash_size_freq, self.entrypoint))  # pylint: disable=no-member
 
 
 class ESPFirmwareImage(BaseFirmwareImage):
@@ -864,7 +864,7 @@ def write_mem(esp, args):
 
 
 def dump_mem(esp, args):
-    f = file(args.filename, 'wb')
+    f = open(args.filename, 'wb')
     for i in range(args.size / 4):
         d = esp.read_reg(args.address + (i * 4))
         f.write(struct.pack('<I', d))
@@ -944,7 +944,7 @@ def make_image(args):
     if len(args.segfile) != len(args.segaddr):
         raise FatalError('Number of specified files does not match number of specified addresses')
     for (seg, addr) in zip(args.segfile, args.segaddr):
-        data = file(seg, 'rb').read()
+        data = open(seg, 'rb').read()
         image.add_segment(addr, data)
     image.entrypoint = args.entrypoint
     image.save(args.output)
@@ -1026,7 +1026,7 @@ def read_flash(esp, args):
     t = time.time() - t
     print('\rRead %d bytes at 0x%x in %.1f seconds (%.1f kbit/s)...'
            % (len(data), args.address, t, len(data) / t * 8 / 1000))
-    file(args.filename, 'wb').write(data)
+    open(args.filename, 'wb').write(data)
 
 
 def _verify_flash(flasher, args, flash_params=None):
@@ -1235,7 +1235,7 @@ def __call__(self, parser, namespace, values, option_string=None):
         for i in range(0,len(values),2):
             try:
                 address = int(values[i],0)
-            except ValueError as e:
+            except ValueError:
                 raise argparse.ArgumentError(self,'Address "%s" must be a number' % values[i])
             try:
                 argfile = open(values[i + 1], 'rb')

python/update.py

@@ -12,7 +12,6 @@ def ensure_st_up_to_date():
 
   panda = None
   panda_dfu = None
-  should_flash_recover = False
 
   while 1:
     # break on normal mode Panda

setup.py

@@ -13,7 +13,7 @@
 import codecs
 import os
 import re
-from setuptools import setup, Extension
+from setuptools import setup
 
 here = os.path.abspath(os.path.dirname(__file__))
 

tests/automated/0_builds.py

@@ -1,4 +1,3 @@
-import os
 from panda import build_st
 
 def test_build_panda():

tests/automated/1_program.py

@@ -1,6 +1,4 @@
-import os
-from panda import Panda
-from .helpers import panda_type_to_serial, test_white_and_grey, test_all_pandas, panda_connect_and_init
+from .helpers import test_all_pandas, panda_connect_and_init
 
 @test_all_pandas
 @panda_connect_and_init

tests/automated/2_usb_to_can.py

@@ -1,5 +1,3 @@
-
-import os
 import sys
 import time
 from panda import Panda
@@ -180,6 +178,6 @@ def test_gmlan_bad_toggle(p):
 @test_all_pandas
 @panda_connect_and_init
 def test_serial_debug(p):
-  junk = p.serial_read(Panda.SERIAL_DEBUG)
+  _ = p.serial_read(Panda.SERIAL_DEBUG)  # junk
   p.call_control_api(0xc0)
   assert(p.serial_read(Panda.SERIAL_DEBUG).startswith(b"can "))

tests/automated/3_wifi.py

@@ -1,5 +1,3 @@
-
-import os
 import time
 from panda import Panda
 from .helpers import connect_wifi, test_white, test_all_pandas, panda_type_to_serial, panda_connect_and_init

tests/automated/4_wifi_functionality.py

@@ -1,8 +1,6 @@
-
 import time
 from panda import Panda
 from .helpers import time_many_sends, connect_wifi, test_white, panda_type_to_serial
-from nose.tools import timed, assert_equal, assert_less, assert_greater
 
 @test_white
 @panda_type_to_serial

tests/automated/5_wifi_udp.py

@@ -3,7 +3,7 @@
 import time
 from .helpers import time_many_sends, connect_wifi, test_white, panda_type_to_serial
 from panda import Panda, PandaWifiStreaming
-from nose.tools import timed, assert_equal, assert_less, assert_greater
+from nose.tools import assert_less, assert_greater
 
 @test_white
 @panda_type_to_serial

tests/automated/6_two_panda.py

@@ -171,7 +171,7 @@ def _test_buses(send_panda, recv_panda, _test_array):
       time.sleep(0.1)
 
       # check for receive
-      cans_echo = send_panda.can_recv()
+      _ = send_panda.can_recv()  # cans echo
       cans_loop = recv_panda.can_recv()
 
       loop_buses = []

tests/automated/helpers.py

@@ -2,13 +2,12 @@
 import sys
 import time
 import random
-import binascii
 import subprocess
 import requests
 import _thread
 from functools import wraps
 from panda import Panda
-from nose.tools import timed, assert_equal, assert_less, assert_greater
+from nose.tools import assert_equal
 from parameterized import parameterized, param
 
 SPEED_NORMAL = 500

tests/black_loopback_test.py

@@ -11,9 +11,6 @@
 import random
 import argparse
 
-from hexdump import hexdump
-from itertools import permutations
-
 sys.path.append(os.path.join(os.path.dirname(os.path.realpath(__file__)), ".."))
 from panda import Panda
 
@@ -33,7 +30,7 @@ def run_test(sleep_duration):
   pandas[0] = Panda(pandas[0])
   pandas[1] = Panda(pandas[1])
 
-  # find out the hardware types  
+  # find out the hardware types
   if not pandas[0].is_black() or not pandas[1].is_black():
     print("Connect two black pandas to run this test!")
     assert False
@@ -66,14 +63,14 @@ def run_test(sleep_duration):
   test_buses(pandas[0], pandas[1], test_array, sleep_duration)
   print("***************** TESTING (1 --> 0) *****************")
   test_buses(pandas[1], pandas[0], test_array, sleep_duration)
-	
+
 
 def test_buses(send_panda, recv_panda, test_array, sleep_duration):
   for send_bus, send_obd, recv_obd, recv_buses in test_array:
     send_panda.send_heartbeat()
     recv_panda.send_heartbeat()
     print("\nSend bus:", send_bus, " Send OBD:", send_obd, " Recv OBD:", recv_obd)
-    
+
     # set OBD on pandas
     send_panda.set_gmlan(True if send_obd else None)
     recv_panda.set_gmlan(True if recv_obd else None)
@@ -92,7 +89,7 @@ def test_buses(send_panda, recv_panda, test_array, sleep_duration):
     time.sleep(0.1)
 
     # check for receive
-    cans_echo = send_panda.can_recv()
+    _ = send_panda.can_recv()  # cans echo
     cans_loop = recv_panda.can_recv()
 
     loop_buses = []
@@ -101,7 +98,7 @@ def test_buses(send_panda, recv_panda, test_array, sleep_duration):
       loop_buses.append(loop[3])
     if len(cans_loop) == 0:
       print("  No loop")
-    
+
     # test loop buses
     recv_buses.sort()
     loop_buses.sort()

tests/black_white_loopback_test.py

@@ -11,9 +11,6 @@
 import random
 import argparse
 
-from hexdump import hexdump
-from itertools import permutations
-
 sys.path.append(os.path.join(os.path.dirname(os.path.realpath(__file__)), ".."))
 from panda import Panda
 
@@ -118,10 +115,10 @@ def test_buses(black_panda, other_panda, direction, test_array, sleep_duration):
 
     # check for receive
     if direction:
-      cans_echo = black_panda.can_recv()
+      _ = black_panda.can_recv()  # can echo
       cans_loop = other_panda.can_recv()
     else:
-      cans_echo = other_panda.can_recv()
+      _ = other_panda.can_recv()  # can echo
       cans_loop = black_panda.can_recv()
 
     loop_buses = []