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

Fixed strandtest.py #258

Merged
merged 5 commits into from
Mar 16, 2018
Merged

Fixed strandtest.py #258

merged 5 commits into from
Mar 16, 2018

Conversation

TheTechromancer
Copy link
Contributor

@TheTechromancer TheTechromancer commented Jan 4, 2018 •
edited
Loading

  1. LEDs now turn off when CTRL+C is pressed.
  2. Removed reference to ws.WS2811_STRIP_GRB, which was not imported and was causing the script to fail. It is not needed for proper function.
  3. Added shebang for our lovely Linux friends.
  4. Fixed some indentation inconsistency.
    EDIT: strandtest.py was failing to run before these modifications

@Gadgetoid Gadgetoid mentioned this pull request Jan 25, 2018
Closed
@Gadgetoid
Copy link
Collaborator

I disagree with 2. - defining and using a strip type serves a purpose for people who tend to hack on code and not touch much documentation. It should probably be fixed and kept as an argument to Adafruit_NeoPixel rather than removed altogether.

Since you've tweaked indentation, you should also replace tabs with Python's conventional 4 spaces. While this sounds like needless pedantry it prevents running into indentation errors when mixing Python code across examples/projects.

Otherwise; good stuff!

Gadgetoid
Gadgetoid requested changes Mar 6, 2018
View reviewed changes
python/examples/strandtest.py Outdated
@@ -11,15 +12,15 @@
import signal
import sys
def signal_handler(signal, frame):
colorWipe(strip, Color(0,0,0))
sys.exit(0)
colorWipe(strip, Color(0,0,0))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation should be intervals of 4 spaces, not tabs. See: https://www.python.org/dev/peps/pep-0008/#indentation

python/examples/strandtest.py
@@ -30,7 +31,6 @@ def opt_parse():
LED_BRIGHTNESS = 255 # Set to 0 for darkest and 255 for brightest
LED_INVERT = False # True to invert the signal (when using NPN transistor level shift)
LED_CHANNEL = 0 # set to '1' for GPIOs 13, 19, 41, 45 or 53
LED_STRIP = ws.WS2811_STRIP_GRB # Strip type and colour ordering
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be imported and preserved to ensure the difference between WS2811 and SK6812 pixels is clear to anyone hacking on this code.

python/examples/strandtest.py Outdated
for j in range(strip.numPixels()):
strip.setPixelColor(j, Color(0,0,0))
strip.show()
exit(1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exit() is a builtin for the the Python repl and shouldn't be used in code. You could use sys.exit() here, but as far as I can tell the code is going to exit at this point anyway so you can omit it altogether.

@Gadgetoid
Copy link
Collaborator

Thank you! This is a step in the right direction for all the supporting Python code. Much appreciated!

@Gadgetoid Gadgetoid merged commit 755390e into jgarff:master Mar 16, 2018
brindosch pushed a commit to hyperion-project/rpi_ws281x that referenced this pull request Apr 5, 2018
@penfold42 @brindosch
Updating from upstream (#7)
6c5ade9 
* Added delay to ensure reset time being met with low led counts (jgarff#157)

* Fix includes as per jgarff#178 (jgarff#189)

* clean up doco and strandtest (jgarff#187)

* updated documentation on GPIO usage
strandtest now defaults to GRB colour order
added version.h to gitignore

* updated python readme

* more README cleanup

* added notes on spi max transfer size

* Fix freeze after running for long time  (jgarff#197)

* Whitespace cleanup

* Move channel_protocol_time so it handles 32 bit Color LEDs.

* Fix a freezing issue when running for a long time.

When current_timestamp overflowed 2^64, rolling back to 0 and ws2811->render_wait_until had not, this caused an incredibly long usleep in ws2811_render.

* Update README.md (jgarff#191)

* Update README.md

* updated spi buffer size location

* Add -lrt link flag for systems with older glibc (jgarff#195)

* Fix 32-bit overflow in get_microsecond_timestamp(). (jgarff#206)

On Raspbian systems, the tv_sec member of struct timespec is 32 bits
wide.  get_microsecond_timestamp() was overflowing in 32-bit
arithmetic on this member.  This causes the time that it returns to
jump ahead by many thousands of years at uptime = 35 minutes, 48
seconds.  (While the starting point for CLOCK_MONOTONIC_RAW is
unspecified, it seems to track system uptime in practice.)

This overflow didn't cause noticeable problems for me--ws2811_render()
skips the sleep when it thinks such a huge amount time has
elapsed--but it could possibly result in rarely-occurring skipped
frames if the WS2811 reset time isn't effectively honored by
ws2811_render().

* Add TOOLCHAIN option for cross compilation (jgarff#213)

Use this option if you want to cross-compile the library.
Typical usage is:

  scons TOOLCHAIN=arm-linux-gnueabihf

* Add -Wextra compile option and adapt code to suppress warnings (jgarff#214)

The changes in code are mostly type casts between int ann unsigned.

* Add shared library rasbpian package builder.  The package is not built by
default.  To generate the package you need:

- The dpkg-deb utility.
- Use "scons deb" to generate the package.

* Merge user-settable gamma correction into master (jgarff#216)

Merge user-settable gamma correction into master

* Bug fixes for Stretch and Compute Module support (jgarff#222)

* Bug fixes

Missing aligned(4) attribute in pcm.h makes the PCM case failing under
Stretch, due to a newer version of gcc (6.x).
Also fixed a bug in check_hwver_and_gpionum, so original Model A is
recognised.

* README, using SPI on RPi3

Added note to change the core frequency to 250 MHz when using SPI on
RPi 3.

* README fix

* Add support for Compute Module 1 and 3

* Fix undefined clock_gettime symbol

* add missing memory allocation for SPI mode (jgarff#231)

* added Ctrl-C handler to turn off all LEDs on exit (jgarff#140)

* added Ctrl-C handler to turn off all LEDs on exit

* added option -c

* Fix for segmentation fault on exit for Python API (jgarff#239)

Remove useless __del__ because the cleanup() function gets called by
atexit().
Remove ws.ws2811_fini() call which causes the SEGFAULT on exit.
Python does not complain about the memory leak in this way.

* Add support for new A+ hardware.

* Change default DMA channel to safe choice (10) (jgarff#253)

add warning on README.md

* Add getBrightness for brightness feedback from the buffer (jgarff#248)

* Instructions to install the Python library (jgarff#254)

Solving issue jgarff#142

* Fix for non-root users & SPI (jgarff#257)

* Fix for non-root users can use SPI

* use DEFINE'd device strings

also add a note to the readme

* Basic Golang Examples (jgarff#256)

* Added golang example

* Added basic README for golang examples. Added os exits for errors during color wipes for golang examples

* Fix the default DMA in python code (jgarff#266)

DMA 5 causes file system errors on PI 3

* Fixed typo in README.md (jgarff#273)

Was perusing recent commits and saw this.

* clarify electrical notes (jgarff#264)

* Fixed strandtest.py (jgarff#258)

* Fixed keyboardinterrupt, indentation inconsistency, & a few other minor bugs.  Added shebang.

* Fixed KeyboardInterrupt.  Again.

* KeyboardInterrupt works now.  For sure.

* Tabs to spaces, removed redundant signal handling

* Removed unused sys module import

* Added Raspberry Pi 3 B+ (jgarff#276)

* Added Pi 3 B+

* Remove unused import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gadgetoid Gadgetoid Gadgetoid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TheTechromancer @Gadgetoid