Added API for gpio get pin by id

[alexandruradovici]

The API modifications and example to use tock/tock#1675.

I have not tested this yet as I don't have any of the supported boards.

[jrvanwhy]

I'm not sure why Travis didn't run (it checks formatting), but I think there should be a space between the colon and usize here. I think the correct command to run to format the code is cargo fmt.

[Woyten]

You can run ./run_all_checks.sh to verify your formatting. Travis is triggered only when bors receives a command.

[alexandruradovici]

I fixed the formatting, I did not know about cargo fmt.

[Woyten]

There is a reason why a gpio_by_id has not been implemented: It enables you get two references to the same GPIO port which you could use to open the same port in read AND write mode at the same time. This is a logical error and, according to the Tock documention, even worse: undefined behavior.

To access different GPIOs in a safe way the idea was to use the iterator Gpios. This iterator could be extended to have a function called next_at(usize) -> Option<Gpio>.

Regarding the example: I do not think it is beneficial to demonstrate every possible API function in an example. In my opinion a unit test would make more sense.

[alexandruradovici]

There is a reason why a gpio_by_id has not been implemented: It enables you get two references to the same GPIO port which you could use to open the same port in read AND write mode at the same time. This is a logical error and, according to the Tock documention, even worse: undefined behavior.

I didn't notice that, thank you.

To access different GPIOs in a safe way the idea was to use the iterator Gpios. This iterator could be extended to have a function called next_at(usize) -> Option<Gpio>.

I implemented the next_at method and a reset method. As next_at allow you to jump over pins, some pins would not be available anymore. I use an array (fixed size, not sure how to deal with this in another way) to mark the used pins. I also added a reset method to be able to reset the counter.

I made a modification to the next method so that it checks if the pins had been previously exported.

Regarding the example: I do not think it is beneficial to demonstrate every possible API function in an example. In my opinion a unit test would make more sense.

How to you see the unit test implemented? I haven't seen any in the gpio.rs file or the examples, but most probably I missing something.

[Woyten]

This is going in a good direction. However, keeping track of each GPIO pin individually will most probably add too much overhead for an embedded setup. A trade-off solution could be:

• pinid < self.curr_gpio => return None or Err(ReverseIterationError)
• pinid >= self.num_gpios => return None or Err(OutOfRangeError)
• otherwise => set self.curr_gpio to pinid and return the Gpio instance

[alexandruradovici]

The problem that I see is that without a reset method, there is no way of getting any pins with a lower index that the requested one. As a user has no idea about what index a pin has, it depends on the mapping in the board/main.rs file, how would you solve that?

One option would be to add a function that returns the index of the pin, so that users can sort them and ask them in order.

[Woyten]

I think I do not quite understand the scenario. Why would you randomly access pins? Wouldn't you just know beforehand which pins to use. Say, you now you need pins 7, 4 and 19, so you would call next_at(4), next_at(7), next_at(19)?

[Woyten]

Wouldn't this reintroduce the soundness problem mentioned before?

[alexandruradovici]

If we keep track of every used pin, no, otherwise yes.

[Woyten]

How to you see the unit test implemented? I haven't seen any in the gpio.rs file or the examples, but most probably I missing something.

We recently added some framework. You could have a look at the single_led_can_be_enabled example. It's probably straight-forward to test that the next_at function works as expected.

[torfmaster]

What is the status of this PR? Is there a consensus about whether or not to merging this PR? Otherwise I would like to close the PR.

[alexandruradovici]

I will close the PR, as the GPIO HIL now allows None pins (tock/tock#1690).