machine/pin: add pin.ConfigureAsInput() and pin.ConfigureAsOutput() helper functions

[deadprogram]

trafficstars

This PR adds the PinConfigInput() and PinConfigOut() helper functions to avoid so much repetition when just using the default values for config:

Before:

	led.Configure(machine.PinConfig{Mode: machine.PinOutput})

After:

	led.Configure(machine.PinConfigOutput())

You can still use the longer form, this is just a shortened form since the defaults are so often used.

[deadprogram]

Any opinions on this PR @aykevl @conejoninja @sago35 anyone? :smile_cat:

[conejoninja]

I like the idea, it's not only shorter (not much) but easier/simpler. Could the code be placed inside machine.go only once? It smells a little that it needs to be copied/pasted in every machine_xxx.go file, we'll need to make sure new devices add those functions too.

To make it even simpler, what about :

led.ConfigureAsOutput()
led.ConfigureAsInput()

[aykevl]

What about pull up and down? It seems to me that those are rather important and also supported on most chips (I believe AVR is an exception and it only supports pull up, not pull down).

[aykevl]

Also, what problem are you trying to solve exactly? Adding PinConfigInput and PinConfigOutput (returning a machine.PinConfig) do not remove a dependency on the machine package.

[deadprogram]

This change is not intended to remove dependency on machine. It is only to simplify the syntax for default values. One benefit is it removes the need to explain "what is an empty struct" for simple examples. The purpose is to streamline the experience for new programmers, as well as people new to the Go language.

In fact, I much prefer @conejoninja suggestion, and I think I will change this PR to incorporate it.

Regarding pull-up vs. pull-down, the proposed change to pin.ConfigureAsInput() should probably do the most likely thing on that hardware. For devs who require more specific control, the current pin.Configure() is still intended to be used. The simplified syntax is only intended as a means for shortcut to the most common default.

[aykevl]

Ah okay, thank you for the explanation.

I'm not entirely convinced that adding more API surface will simplify things, to be honest. However, the current API is a bit more complicated than necessary so maybe we can simplify something? This might break existing code depending on what we do.

For example:

pin.Configure(machine.PinOutput)
pin.Configure(machine.PinInput)
pin.Configure(machine.PinInputPullup)

And if we later need more options (which so far don't seem to be necessary), they can be implemented with extra methods, such as:

pin.SetDriveStrength(10) // 10mA drive strength

[aykevl]

Although, maybe the suggestion from @conejoninja is better as it also allows using the pins in an interface. For example, a hypothetical button driver could use this interface:

type Pin interface {
    ConfigureAsInputPullup()
    Get() bool
}

It still looks a bit ugly to me to have four methods for essentially one thing (ConfigureAsInput, ConfigureAsOutput, ConfigureAsInputPullup, ConfigureAsInputPulldown) but it would work neatly in an interface. And it doesn't break all the TinyGo code that already exists.

[deadprogram]

Although, maybe the suggestion from @conejoninja is better as it also allows using the pins in an interface.

That right there does achieve both the simplification I am seeking, and also gives us an interface for testing.

So do we want to go with this:

type Pinner interface {
    ConfigureAsInputPullup() error
    ConfigureAsInputPulldown() error
    ConfigureAsOutput() error
    Get() bool
    Set(bool) error
}

or this:

type Pinner interface {
    ConfigureAsInput() error
    ConfigureAsOutput() error
    Get() bool
    Set(bool) error
}

Where ConfigureAsInput() will be pulldown or pullup depending on what is the default for that hardware.

What do you think?

[conejoninja]

I'm more inclined to the simplest ConfigureAsInput() if possible. I didn't see any "Arduino" tutorial that makes the difference between pull-up / pull-down, at least in entry level.

[deadprogram]

Seems like pullup is more common then pulldown. We could default to that for ConfigureAsInput() and document to use the longer syntax if you need a pulldown?

[aykevl]

What do you mean, default? There are three possible ways to configure an input: with a pulldown, a pullup, and floating. machine.PinInput configures the input as a floating input and is what people probably expect (a pulldown or pullup can have side effects you don't want). So, there is no "correct" or "default" pull, except that AVR does not usually support pulldown. The pull mode is simply an extra added resistor to VCC or GND, floating means that there is no such register and something else needs to pull the line up or down (such as an external resistor or an output of another chip).

Regarding the interface: I would avoid making one shared interface type. Instead, I think every driver should define the subset that the driver needs as this can vary greatly. One driver might only need ConfigureAsOutput and Set, while another might also need to configure a pin as an input.

[deadprogram]

The interface will be defined in the consumer aka the drivers repo, not here.

I just pushed a commit to this branch with the ConfigureAsOutput() and ConfigureAsInput() functions. Based on what you were saying, perhaps they should default to floating inputs instead of pullup?

[deadprogram]

OK I have rebased and squashed commits. I think this is now ready to go, unless there is additional feedback.

[deadprogram]

@aykevl any other feedback on this PR?

[deadprogram]

Closing, since we decided not to do this.