Update add-on boards to use BOARD numbering, close #349

[bennuttall]

As suggested by @lurch in #349, this branch updates add-on boards to use BOARD numbering so that the right BCM numbers are elected regardless of Pi model.

[martynwheeler]

Hi,

Thank you for this. How do I use this branch?

[ukBaz]

Hi @martynwheeler

Are you asking for the git commands to get this branch? e.g:
https://stackoverflow.com/questions/1783405/how-do-i-check-out-a-remote-git-branch
git checkokut 349-board-numbering-addons

Or are you seeking clarification about the different pin Pin Numbering schemes? e.g:
https://gpiozero.readthedocs.io/en/stable/recipes.html#pin-numbering

[martynwheeler]

Hi,
Sorry for not being clear. It was the former that I was looking for. Is it likely that these changes will be merged into the master?

Thanks, Martyn

[martynwheeler]

Sorry for the questions,
Do I have to clone the git repository and then install the branch? I am not very au fait with using git and am slightly confused?? Or is it easier to wait until this branch is merged?

[lurch]

There's quite good docs at https://gpiozero.readthedocs.io/en/stable/development.html#development-installation 😉

[martynwheeler]

Thanks,
Not really sure that I understand the process enough, I don't want to develop and not sure how to use a virtual environment. I think I will just have to wait until it is merged and just use something else in the meantime.

EDIT: I may try and follow the instructions to see if it works this afternoon

[martynwheeler]

Hmm, I tried the first couple of things and then got this:

$ git clone https://github.com/RPi-Distro/python-gpiozero.git
Cloning into 'python-gpiozero'...
remote: Enumerating objects: 111, done.
remote: Counting objects: 100% (111/111), done.
remote: Compressing objects: 100% (77/77), done.
error: RPC failed; curl 56 GnuTLS recv error (-9): Error decoding the received TLS packet.
fatal: the remote end hung up unexpectedly
fatal: early EOF
fatal: index-pack failed

[bennuttall]

That looks like your internet connection.

I've made a release you can try:

sudo pip3 install https://bennuttall.com/files/gpiozero-1.5.2b349-py3-none-any.whl

[martynwheeler]

Thank you, I have to go and teach a lesson now. Will try later.

[martynwheeler]

@bennuttall - thanks for the pointer (Virgin Media's internet safety feature was blocking git). Okay, I have followed the instructions and installed this branch in a virtual environment and it works perfectly! Thank you. Will this branch be merged into the master in the near future or should I install this branch on my system?

Once again, @lurch and @ukBaz thanks for guiding me through the process.

[bennuttall]

Good to know it works. @lurch is this what you had in mind? I'll try and check with Dave if he has a mo.

Seems like a reasonable approach to take for all physical boards really.

[lurch]

@lurch is this what you had in mind?

I can't quite remember, given that I originally opened that issue 4 years ago 😉 But yeah the change here looks reasonable.
Thanks to @martynwheeler for testing and confirming that it works on your old Pi.

Seems like a reasonable approach to take for all physical boards really.

Yeah IMHO if you're gonna be doing this it makes sense to do it for all add-on boards (for consistency).

[bennuttall]

Yeah IMHO if you're gonna be doing this it makes sense to do it for all add-on boards (for consistency).

Well, that was a dull way to spend the last hour. Done!

One thing that annoys me is that the way the input/output pins are made available with the new Pibrella class:

In [1]: from gpiozero import *                                                                           

In [2]: pb = Pibrella()                                                                                  

In [3]: pb.inputs.a                                                                                      
Out[3]: 'BOARD21'

But I guess they still work as intended:

In [4]: btn = Button(pb.inputs.a, pull_up=False) 
   ...: led = LED(pb.outputs.e)

So it's only really if somebody prints them out that it could confuse people... bit of an edge case though!

One question: what does LED('BOARD2') or god forbid Energenie() do on a compute module? Does it matter? 😆

[bennuttall]

Oh jesus... find and replace 😱

[codecov-io]

Codecov Report

Merging #860 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #860      +/-   ##
==========================================
- Coverage   79.58%   79.58%   -0.01%     
==========================================
  Files          23       23              
  Lines        4517     4516       -1     
  Branches      656      656              
==========================================
- Hits         3595     3594       -1     
  Misses        866      866              
  Partials       56       56              

Impacted Files	Coverage Δ
gpiozero/boards.py	99.79% <100.00%> (-0.01%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 497b716...d889897. Read the comment docs.

[lurch]

I wonder if it might be worth leaving the tests using the GPIO numbers, as that provides a vague sanity-check that the translations between BOARD numbers and GPIO numbers is working correctly?
EDIT: Ignore that, my mistake!

One thing that annoys me is that the way the input/output pins are made available with the new Pibrella class

Hmmm, that's a shame - I kinda assumed that specifying a BOARD number when constructing an object would automatically convert it back into the internal GPIO numbers. But thinking about it, perhaps constructing e.g. a LED with a BOARD number and having it print as a GPIO number, would be even more confusing?

Oh... now having looked at the code in more detail 🔍 I see that it's because pb.inputs is just a namedtuple, and a GpioZero object hasn't been created yet. But I think if you printed out btn.pin in your example then you'd see the GPIO number?

One question: what does LED('BOARD2') or god forbid Energenie() do on a compute module?

Looking at https://github.com/gpiozero/gpiozero/blob/master/gpiozero/pins/data.py#L1272 and https://github.com/gpiozero/gpiozero/blob/master/gpiozero/pins/data.py#L320 I think that 'BOARD2' on a compute module would get converted to 'EMMC DISABLE N' and then "raise PinInvalidPin('%s is not a valid pin spec' % spec)"

Does it matter? 😆

Probably not ;-)

[bennuttall]

Eugh it seems silly to worry about compute module but if you think about a non-industrial hacker type playing around with one - what would they expect? Wire up e.g. en Energenie using the right BCM pins, they would expect it to work.

Let's say we make it possible for that to work - some workaround that we document in the FAQs - what would that look like?

My initial thought would be allowing an override to pins in the constructor:

class SomeHat(LEDBoard):
    def __init__(self, pins=None, pwm=False, pin_factory=None):
        if pins is None:
            pins = (2, 3, 4, 5)
        elif len(pins) != 4:
            raise GPIOPinErrorWhatever()
        super().__init__(*pins, pwm=pwm, pin_factory=pin_factory)

Instead of what this class would look like now:

class SomeHat(LEDBoard):
    def __init__(self, pwm=False, pin_factory=None):
        pins = (2, 3, 4, 5)
        super().__init__(*pins, pwm=pwm, pin_factory=pin_factory)

An entirely different approach to all of this would be: instead of using BOARD numbers in classes, we call a function that converts physical pin numbers to BCM numbers for the original Pi, but leaves them alone for everything else, e.g:

def pin(n):
    # where n is the board number
    rev = pi_info().revision
    if rev == ORIGINAL_PI_REV:
        return ORIGINAL_PI_PINS[n]
    return n

This is ridiculous 😆

[lurch]

def __init__(self, pins=None, pwm=False, pin_factory=None):
Provided that the pins parameter is moved to be last in the list (for backwards compatibility) I think an approach like that might work. This would also be beneficial for people using things like https://shop.pimoroni.com/products/mini-black-hat-hack3r to connect multiple add-on boards to their Pi at once 😃

Although perhaps if a pins parameter is given it should be a dict instead of a tuple ?

[bennuttall]

It would be better all-round if pwm was keyword-only :)

Good point re: BHH

In a way, adding pins could be a separate solution to this. I think it's worth considering regardless. Gets a bit tricky with the "shape" of the value...

What do you think about the other approach? We could do both 😆

[lurch]

It would be better all-round if pwm was keyword-only :)

I think I once remember seeing @waveform80 say that Python3 allows keyword-only arguments (?), but that we couldn't use this yet because GpioZero supports py2 and py3. But if we're going to be dropping py2...

What do you think about the other approach? We could do both laughing

Given how complex this is getting, I think we ought to stick to just one way of doing this 😉

Gets a bit tricky with the "shape" of the value...

Do you mean checking that the required numbers of pins are provided? IMHO it makes sense for this to be an all-or-nothing thing - either the pins is left at None (the default) or all pins must be provided, with anything else being an error.
We could have e.g. hat = TrafficHat(pins={'red': 2, 'amber': 3, 'green': 4, 'button': 5, 'buzzer': 6}). And extending this back to the ComputeModule question I think that'd even work with hat = TrafficHat(pins={'red': 'BOARD9', 'amber': 'BOARD11', 'green': 'BOARD15', 'button': 'BOARD17', 'buzzer': 'BOARD21'}) 😆

[bennuttall]

I think I once remember seeing @waveform80 say that Python3 allows keyword-only arguments (?), but that we couldn't use this yet because GpioZero supports py2 and py3. But if we're going to be dropping py2...

Yeah, it would be good to enforce keyword-only arguments (where appropriate) post- dropping Python 2.

Given how complex this is getting, I think we ought to stick to just one way of doing this

I see this as two separate things:

• specifying pins in board definitions so that they work on original pis too
• allowing people to override pins for boards (for any reason)

Do you mean checking that the required numbers of pins are provided?

I meant where the board is complex enough to have a nested "shape" (i.e. .value is not just a flat tuple or dict) like the JamHat whose value shape is:

JamHatValue(
    lights_1=LEDBoardValue(red=0, yellow=0, green=0),
    lights_2=LEDBoardValue(red=0, yellow=0, green=0),
    button_1=0,
    button_2=0,
    buzzer=None
)

[lurch]

I see this as two separate things:

Yeah, I meant that I don't think there's much value in investigating your "instead of using BOARD numbers in classes, we call a function that converts physical pin numbers to BCM numbers for the original Pi, but leaves them alone for everything else" suggestion - IMHO it feels far too special-case and error-prone. (And isn't "a function that converts physical pin numbers to BCM numbers" exactly what BOARD numbering does anyway? 😉 )

I meant where the board is complex enough to have a nested "shape" (i.e. .value is not just a flat tuple or dict) like the JamHat

Ah, so you're asking whether a board with a "shaped" .value, should also have a "shaped" pins parameter in its constructor? I personally don't think there's any value in that - given that if a custom pins parameter (if used) will only ever be a one-off thing, IMHO it makes sense to keep pins as a flat dict? Remember that all it's telling us is how the user has re-wired the connections to their add-on board (using either the ComputeModule or something like a BlackHatHacker).
So I'd suggest something like:

hat = JamHat(pins={'red1': 2, 'yellow1': 3, 'green1': 4, 'red2': 5, 'yellow2': 6, 'green2': 7, 'button1': 8, 'button2': 9, 'buzzer': 10})

(with the JamHat constructor than checking that pins is None or pins.keys() == set(('red1', 'yellow1', 'green1', 'red2', 'yellow2', 'green2', 'button1', 'button2', 'buzzer')) )

But perhaps it's worth keeping this PR as just "specifying pins in board definitions so that they work on original pis too" and then creating a separate issue / PR for "allowing people to override pins for boards (for any reason)" ?

Hmmmm2, perhaps we could allow a "special value" to be supplied to the pins init-parameter to allow a user to indicate that although they're using a ComputeModule, they've actually wired up their GPIO numbers the same as on a regular Pi B+ ? (which I guess is probably the case for a number of ComputeModule "host boards"?) hat = JamHat(pins='USE_BPLUS_MAPPING') ?

P.S. Slightly off-topic question, but is there a reason why JamHat uses two LEDBoard instances rather than two TrafficLights instances?

edit: fixed the dict syntax

[bennuttall]

I personally don't think there's any value in that - given that if a custom pins parameter (if used) will only ever be a one-off thing

True. We could use the dict declaration ourselves, so instead of:

super.__init__(lights=TrafficLights(red=2, amber=3, green=4), button=5)

we do:

if pins is None:
    pins = {red: 2, amber=3, green=4, button=5}
super.__init__(lights=TrafficLights(red=pins['red'], amber=pins['amber'], green=pins['green']), button=pins['button'])

That way we pave the way for copy/paste/edit.

Hmmmm2, perhaps we could allow a "special value" to be supplied to the pins init-parameter to allow a user to indicate that although they're using a ComputeModule

That makes sense :)

P.S. Slightly off-topic question, but is there a reason why JamHat uses two LEDBoard instances rather than two TrafficLights instances?

I don't know! I'd have to check the PR. Probably a mistake by overthinking it. I know what you're thinking, that sounds really out of character, but I guess sometimes these things happen.

[lurch]

if pins is None:
    pins = {'red': 2, 'amber': 3, 'green': 4, 'button': 5}
super.__init__(lights=TrafficLights(red=pins['red'], amber=pins['amber'], green=pins['green']), button=pins['button'])

Nice! Except we'd obviously be using 'BOARD' numbers rather than GPIO numbers in the second line 😉

[martynwheeler]

Hi,
Just wondering when this will be merged into the main branch? Thank you, Martyn

[bennuttall]

Not sure yet - think it needs a rethink. And we're in the middle of another project at the moment.

[martynwheeler]

<!-- /* Font Definitions */ @font-face {font-family:"Cambria Math"; panose-1:2 4 5 3 5 4 6 3 2 4;} @font-face {font-family:Calibri; panose-1:2 15 5 2 2 2 4 3 2 4;} /* Style Definitions */ p.MsoNormal, li.MsoNormal, div.MsoNormal {margin:0cm; margin-bottom:.0001pt; font-size:11.0pt; font-family:"Calibri",sans-serif;} a:link, span.MsoHyperlink {mso-style-priority:99; color:blue; text-decoration:underline;} .MsoChpDefault {mso-style-type:export-only;} @page WordSection1 {size:612.0pt 792.0pt; margin:72.0pt 72.0pt 72.0pt 72.0pt;} div.WordSection1 {page:WordSection1;} -->No problem, thanks for the update. Sent from Mail for Windows 10 From: Ben NuttallSent: 24 June 2020 13:29To: gpiozero/gpiozeroCc: martynwheeler; MentionSubject: Re: [gpiozero/gpiozero] Update add-on boards to use BOARD numbering, close #349 [WIP] (#860) Not sure yet - think it needs a rethink. And we're in the middle of another project at the moment.—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or unsubscribe. 

[waveform80]

These could just go in pins?

[waveform80]

Okay, read through the thread: yes, this will break on compute modules (1, 3, and 3+). However, I'm not fussed. Given CM4 is now out and (due to the various needs of the high-speed interfaces) has reverted to a basically "classic" GPIO layout (at least on the dev board), and that CM1/3/3+ users will be a tiny niche of gpiozero users, I'm fine with merging this as is. Any objections @bennuttall ?