Conversation
Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
Signed-off-by: Geoff Gustafson <geoff@linux.intel.com>
[gpio] Fix bugs in GPIO sample code
…board Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
|
@grgustaf @pfalcon @nagineni @brianjjones @jimmy-huang @poussa please check |
Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
grgustaf
left a comment
grgustaf
left a comment
There was a problem hiding this comment.
I like these changes and am already implementing to them so I hope you adopt them. :) +1
board/README.md
Outdated
| <a name="board"></a> | ||
| ### The `Board` interface | ||
| Represents a hardware board. | ||
| Represents a hardware circuit board such as Arduino 101. |
| -------------- | ||
| AIO functionality is exposed by the [`AIO`](#aio) object that can be obtained by using the [`aio()`](./README.md/#aio) method of the [`Board` API](./README.md/#board). See also the [Web IDL](./webidl.md). | ||
| <a name="apiobject"></a> | ||
| ### The AIO API object |
There was a problem hiding this comment.
Maybe we should name this ADC since there's no output? We've called it AIO for a long time in ZJS but maybe it should change.
There was a problem hiding this comment.
In some boards DAC is supported, but it's rare. I will check again.
board/gpio.md
Outdated
| ### The GPIO API object | ||
| When requiring `"gpio"`, the following steps are run: | ||
| - If there is no permission for using the functionality, throw `SecurityError`. | ||
| - If the AIO functionality is not supported on the board, throw `"NotSupportedError"`. |
board/i2c.md
Outdated
| ### The I2C API object | ||
| When requiring `"i2c"`, the following steps are run: | ||
| - If there is no permission for using the functionality, throw `SecurityError`. | ||
| - If the AIO functionality is not supported on the board, throw `"NotSupportedError"`. |
board/pwm.md
Outdated
| ### The PWM API object | ||
| When requiring `"pwm"`, the following steps are run: | ||
| - If there is no permission for using the functionality, throw `SecurityError`. | ||
| - If the AIO functionality is not supported on the board, throw `"NotSupportedError"`. |
board/spi.md
Outdated
| ### The SPI API object | ||
| When requiring `"spi"`, the following steps are run: | ||
| - If there is no permission for using the functionality, throw `SecurityError`. | ||
| - If the AIO functionality is not supported on the board, throw `"NotSupportedError"`. |
board/uart.md
Outdated
| ### The UART API object | ||
| When requiring `"uart"`, the following steps are run: | ||
| - If there is no permission for using the functionality, throw `SecurityError`. | ||
| - If the AIO functionality is not supported on the board, throw `"NotSupportedError"`. |
There was a problem hiding this comment.
AIO -> UART
Or maybe we should remove these and just say 'functionality'. :)
board/gpio.md
Outdated
| | --- | --- | --- | --- | --- | | ||
| | `pin` | String or Number | no | `undefined` | board name for the pin | | ||
| | `pin ` | String or Number | no | `undefined` | pin name | | ||
| | `mapping` | String | no | `"system"` | pin mapping | |
There was a problem hiding this comment.
I wonder if it's better to just requires that namespaces unambiguously disjoint. For the Zephyr case at present the OS format is "GPIO_0.15" - they always contain a '.' while board pin names wouldn't. So I have no trouble telling them apart and find that preferable to checking the 'mapping'. That seems easier for the user, and then the impl is free to add other namespaces w/o being explicit about it (e.g. "Arduino"). The implementation has to document it anyway.
There was a problem hiding this comment.
This would be too Zephyr-specific. In general, I wanted to record the developer choice (or whether to have a preference at all) whether they want to look at Zephyr docs or the board labels.
As for implementation docs, that's better avoided: then those should be spec'd and included in board-specific APIs in iot-js-api (and Zephyr.js also) -- which we also wanted to avoid.
Otherwise, in general, implementations could do the mapping themselves, so in Zephyr case instead of saying GPIO_9.15 they could just say 15 as well - but then that would require an iot-js-api specific doc, which we wanted to avoid.
There was a problem hiding this comment.
I don't mean that this spec would specify those formats, I'm just saying this spec could require that if an implementation allows a different (e.g. OS-specific) format of pin naming, they do it in a way that is distinguishable from the system-specific mapping, so that a "mapping" doesn't need to be specified.
There was a problem hiding this comment.
Such a requirement is very hard to make normative, and it is usually discouraged. If the driving concern is to get rid of the mapping property, then we can get rid of it until it proves to be necessary, but then the pin names become completely opaque to the app code, platform dependent, but without a specified policy, that is, "if you assume these strings have a meaning, you are on your own to find their docs".
Let's suppose we get rid of mapping. We could still require implementing board namespaces, i.e. support for the labels that are printed on the board. So the developer would have the guarantee that it's enough to look at the board, and those names would work in this API.
I agree that would also be equally hard to make normative, although it would be a nice-to-have feature.
So let's go by middle ground and remove mapping with the note above about de-scoping pin name semantics from this API.
board/gpio.md
Outdated
| | [`read()`](#read) | synchronous read | | ||
| | [`write()`](#write) | synchronous write | | ||
| | [`close()`](#close) | close the pin | | ||
| The `pin` property is either a number or string, with values defined by the OS or board documentation. The default valus is `undefined`. |
| - Otherwise if `options` is not defined, let `init` be a [GPIOOptions](#gpiooptions) dictionary with all properties taking the default value. | ||
| - If `port` is a number or string, run the following sub-steps: | ||
| * If `mapping` is defined, match `port` to the supported GPIO port names in the pin namespace specified by `mapping`. If not found, throw `InvalidAccessError`. | ||
| * Otherwise if `mapping` is not defined, search `port` first in the OS namespace, then in board namespace. If both fail, throw `InvalidAccessError`. |
There was a problem hiding this comment.
If it's okay to just require that they be disjoint then it requires less work for the implementation to enforce the mappings if the field is present. If we can find any match, we know we're good.
There was a problem hiding this comment.
I am not sure that requirement could be respected in general. But I will think about this.
There was a problem hiding this comment.
We could create an issue to track this.
Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
| The `mode` property MUST take the value `"in"` or `"out"`. The default value is `"out"`. | ||
|
|
||
| The `pin` property inherited from [`Pin`](./README.md/#pin) can take values defined by the board documentation, usually positive integers. | ||
| The `activeLow` property tells whether the pin value 0 means active. If `activeLow` is `true`, with `value` 0 the pin is active, otherwise inactive. For instance, if an actuator is attached to the (output) pin active on low, client code should write the value 0 to the pin in order to activate the actuator. The default value is `false`. |
There was a problem hiding this comment.
In the past, I used boolean values for setting the pins. And this worked with the activeLow property because if you said write(true) it would set the pin high for active high and low for active low. If we're instead using 0's and 1's as I admit seems to be the way other APIs have gone, then I'm not sure it makes sense to even mention activeLow. Based on your definition here I don't think you mean for it to have any programmatic effect. If they set a pin to "1" we will make it high, and the meaning depends on the pin but it doesn't actually matter what this activeLow value is set to.
For the moment I think I will commit the change to numbers in ZJS, but still leave the behavior that if "activeLow" is set, then ZJS will toggle the polarity of the bit as it writes or reads it. But we should come to agreement on this and I'll fix it up to reflect the decision. :)
There was a problem hiding this comment.
I am sure activeLow is not meant to change values on the fly. In this API activeLow is an information about the pin, not something that alters its behavior. That reflects how boards work: you read the doc, figure out which pin is active low, and you control the logic so that it does what you mean.
If we use write, then we should write the value we provide, regardless of semantics. So activeLow is only a configuration option (when possible), and then the developer knows that if it's true, then from now on when we write 0, it means setting(=activating) the pin.
Actually you are right that we don't strictly need activeLow (it's just a kind of semantic tag on the pin), when we use write().
If we wanted to have the change-on-the-fly behavior, then we should have activeLow, set() (or activate) and reset() (deactivate) methods.
The question is which set of API methods do we choose: write() (with activeLow being informative), or set()/reset() (with activeLow being effective). We can even choose all of them (that is how I started), but I felt it's too confusing. The API promise is simple: when we write() 0, then 0 is written. The code can figure out what that means by configuring (and reading) the value of activeLow (or consulting the board doc).
There was a problem hiding this comment.
Now that this is clarified - activeLow is a "no-op" and since we removed it as a readable property of pin, there's no point at all. The user at present just needs to pay attention themselves to which pins are active low or high.
Real world example is that two of the onboard LEDs on Arduino 101 are active low, one is active high. So when I had my "set/reset" type API you were able to configure the pins so that "true" always meant "LED on" which was kind of nice. But not a very important issue to solve, I'm fine with this.
But they might as well put a code comment // this pin is active low vs. setting an unused property on the init object.
There was a problem hiding this comment.
But we could still use activeLow for configuring a pin, where possible. And we could keep it as a read-only property.
The question is whether is there a use case for the set/reset API, and you've just given one. Should I include them?
Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
2 participants
To help constrained implementations,
Also, since Zephyr supports a number or boards with own pin mapping, introduce support for OS pin namespace and board namespace. Apps may specify the namespace for pin names. For now the default namespace is board.