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

BB Blink demo is misleading and its problems poorly documented - fix included #505

Closed
eric-s-raymond opened this issue Mar 29, 2018 · 6 comments

Comments

@eric-s-raymond
Copy link

I modified the blink demo to be clear about what it''s toggling and when, and what its prerequisites are.

/*
 * blinktest - blink the USR1 LED on the Beaglebone once a second.
 *
 * usr1 is the blue LED third from the Ethernet port in the group
 * of four.  Normally it goes on when the SD is being accessed.
 * While this is running expect it to blink on and off with a
 * two-second cycle.
 *
 * Note: you must be running as root or be in the gpio group for
 * this to work.
 */
package main

import (
        "time"

        "gobot.io/x/gobot"
        "gobot.io/x/gobot/drivers/gpio"
        "gobot.io/x/gobot/platforms/beaglebone"

	"log"
)

func main() {
	log.Printf("Blink demo\n")
	pin := "usr1"
        beagleboneAdaptor := beaglebone.NewAdaptor()
        led := gpio.NewLedDriver(beagleboneAdaptor, pin)

        work := func() {
                gobot.Every(1*time.Second, func() {
			log.Printf("Toggling %s", pin)
                        led.Toggle()
                })
        }

        robot := gobot.NewRobot("blinkBot",
                []gobot.Connection{beagleboneAdaptor},
                []gobot.Device{led},
                work,
        )

        robot.Start()
}

This solves several specific problems:

  1. As is, the demo has no visible result unless you happen to have an LED hooked to the P9_12 pin.

  2. It can silently fail due to the permissions issue.

  3. User doesn't know where to look or what success looks like.

Not mentioned in the comment: Whatever library code this demo is using fails to restore the default behavior of blinking on mmc0 access - instead it's not triggerred by anything, That's a bug.

@eric-s-raymond eric-s-raymond changed the title Blink demo is misleading and its problems poorly documented - fix included BB Blink demo is misleading and its problems poorly documented - fix included Mar 31, 2018
@deadprogram
Copy link
Member

Hi @eric-s-raymond thanks for looking at this. A couple comments:

  • there is a separate example for using the usr LED here https://github.com/hybridgroup/gobot/blob/master/examples/beaglebone_blink_usr_led.go
  • We should probably add a comment somewhere in the example that mentions the need to connect an LED to that pin.
  • There should not be a permissions issue if you are running the latest BBB Debian OS, as mentioned here: https://gobot.io/documentation/platforms/beaglebone/ Are you running under a different user account than debian on the BBB?
  • Gobot does not do anything to restore other programs that were previously running using the usr LEDs, I'm not too sure how to do that, or if we even really should be. Any further info on that would be appreciated. In the meantime, we should add info to the docs page, and possibly use a different usr LED for the example. What do you think?

@eric-s-raymond
Copy link
Author

eric-s-raymond commented Apr 1, 2018

I am aware that you have a separate example for usr1. Part of what I am trying to convey is that the "main" demo blinking P9_12 is poorly written as is and should be replaced with an example that blinks a visible LED and provides an indication in its output of when the LED should be blinking.

What is the point of a demo that doesn't demo without special attached equipment when you have one that works with the bare board?

Yes, I was running under a development account I created. I am aware that the debian account is in group gpio. It is good practice to inform the user up front of preconditions like that. The comment I added was as important as the minor code modification.

I don't think restoring the default LED binding is very important as long as warning that it will be left unbound until the next reboot is added to the header comment. Again, if you do not inform users of these things they are likely to be surprised, confused, and have unnecessary problems

I chose usr1 because in normal operation on the BB (as opposed to reflashing) the SD is only accessed if the user does so explicitly. It is thus the least ambiguous pin to use as a success indicator

The incoming merge request addresses all these issues. I don't know how to change the doc page in the repository; if I did, I'd replace the beaglebone_blink_usr_led.go with a link to the improved version.

@gen2thomas
Copy link
Collaborator

Fully agree, that a brief description for all examples will improve the usability. For me - I mostly add a "Println("now on")" or whatever if the example is not working for the first time, just to start investigating a wiring mistake. IMO this is also a good experience for new users, to learn how to investigate such problems.

So I would not add additional output to console by default for simple examples like LED or buzzer. But more important for me would be an output in case of internal errors - most examples just drop it. @eric-s-raymond what do you think about? Your provided change do not check the result of led.Toggle().

@eric-s-raymond
Copy link
Author

Sorry, that was 5 years ago now and I've forgotten the context.

@gen2thomas
Copy link
Collaborator

Yes, a quite long delay 😄 . Anyway thanks for the fast response!

@deadprogram what do you think about:

  • add a brief description for all examples
  • do not drop errors in examples

Thanks Thomas

@gen2thomas gen2thomas self-assigned this Jul 7, 2023
@gen2thomas
Copy link
Collaborator

I'm going to close this issue, because newer examples contains some wiring information, so the user can have a look and will see how it works in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants