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

Support for Ping (pulseIn Firmata) and other improvements #45

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

NeoPolus
Copy link

  • Support for pulseIn compatible Firmata, needed for ultrasonic ranging sensors that use a pulse to measure distances: the new "ping" method to the Pin class returns the echo time. An utility function to convert to distance was also added.
  • Improved the Board class, now it supports the Python "with" statement to be able to automagically manage the Iterator thread (autostarts the iterator, and takes care to call board.exit()).
  • Better support for working with timeouts (useful when the communication with the board hangs -thought that should no longer happen if we use the new 'with' support-).

This for example allows to use the timeout feature, added on tino#38 to the Board constructor, from the helper
function.
Previously some timeouts (when reading from the board and not enought
data was available) would raise a
"TypeError: ord() expected a character, but string of length 0 found"
exception, that is not very useful to know what's happening.

Now it will raise a
"IOError: Failed to read data from port, read N bytes expected M bytes"
that is easier to understand and shows more information.
This enables using HC-SR04 ultrasonic ranging sensors easily
(see http://www.micropik.com/PDF/HCSR04.pdf).

Note: Requires pulseIn compatible Firmata in the arduino board
      (see https://github.com/jgautier/arduino-1/tree/pulseIn).
(To use with ultrasonic ranging sensors).
This will avoid problems with the exit() method not being called
(the __del__ exit() statement was not always called! causing the
connection with the board to hang if an Iterator thread was running).

Also it simplifies the code, as an Iterator will be managed
automagically for the with block.

board = Arduino('/dev/...')
with board: # an Iterator gets started automagically.
    board.get_pin('d:8:i').read() # Works! as an Iterator is running.
    # board.exit() gets called automagically and the Iterator stops
    # after this line.
Use the same pin number in the text as
in the sample HC-SR04 diagram, and show
the units used.
"The significance of this flag is that the entire Python
program exits when only daemon threads are left."

This way Python won't hang at exit, but will just warn of
an exception at shutdown instead.

Anyway it's still better to call ``Board.exit()`` or use
a ``with board: ...`` block to guarantee proper shutdown
and avoid this warning.
@tino
Copy link
Owner

tino commented Oct 29, 2016

Hi, thanks for your extensive pull request. However, it is usually easier to implement features if they are small and contained. My reaction on these three proposals:

  1. I can't find this anywhere in the https://github.com/firmata/arduino library, is that correct? Because I would like to keep this a lean library, and not add code for every different sensor out there. Implementing this should be fairly easy by extending the Board class in your own code. Or did you run into things that made this hard?

  2. I really like the idea of running in a with block! However, to make it less "automagically" as you describe it somewhere I would be in favour of the following syntax, which is more explicit about what's happening:

    with Iterator(board):
        ...

    Which is probably easiest implemented with an _is_running attribute on the Iterator.

  3. Instead of raising a different error, wouldn't it be better to check for the length of self.sp.read() before passing it into ord alltogether? Or does this problem only occur when improperly exiting?

If you would like to move forward with one or more of these proposals, could you please do so in separate pull requests?

Also, please take PEP8 (https://www.python.org/dev/peps/pep-0008/) into account (use 4 spaces instead of tabs, etc.). I use flake8 to check this.

@youssefhabri
Copy link

So, what's the status on this? any chance that pyFirmata will get ping support at some point?

@michaellee8
Copy link

This one would be really helpful, any chance we can provide pulse in through a special flag? without pulsein ultrasonic is not usable in firmata.

@Kakcalu13
Copy link

What can we do to make this PR successful?

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.

5 participants