[RSDK-1008] Add GPIO support for Jetson Orin #1810
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
viambot
added
the
safe to test
edaniels
suggested changes
Jan 27, 2023
|
|
||
| // This helps implement the board.GPIOPin interface for ioctlPin. | ||
| func (pin *ioctlPin) SetPWM(ctx context.Context, dutyCyclePct float64, extra map[string]interface{}) error { | ||
| return errors.New("PWM stuff is not supported on ioctl pins yet") |
There was a problem hiding this comment.
will it be possible to support it in ioctl?
There was a problem hiding this comment.
Eric and I discussed: the answer isn't very clear. By default, it doesn't seem like ioctl has PWM capabilities already built out, but there are folks online who have created such an interface (e.g., here's BeagleBone instructions). How hard would this be to generalize to other boards? I don't know. It's outside the scope of the current ticket/PR, and Eric's question was mere curiosity and not a blocker.
viambot
added
safe to test
|
I'm gonna go resolve merge conflicts, but in the meantime I think I've addressed all of Eric's feedback. Things still work great on the Jetson Orin (GPIO can output low or high, or read low or high signals as input). I still want to test in further detail on a BeagleBone before merging because this does change all the boards formerly referred to as commonsysfs (now genericlinux). but I'm not expecting any surprises/trouble from that. |
penguinland
commented
Jan 31, 2023
…make internal things private
…ut requires copying
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
|
I think for now it's okay but if we start seeing any more divergence in behavior that makes generalization harder. We can freeze this model and add |
stevebriskin
approved these changes
Feb 3, 2023
penguinland
added a commit
that referenced
this pull request
Feb 3, 2023
This is a follow-up to #1810. Data is taken from https://github.com/NVIDIA/jetson-gpio/blob/master/lib/python/Jetson/GPIO/gpio_pin_data.py#L53-L80. I can't test this out because the Jetson Orin NX hardware is not yet on the market (we have the higher-end Orin AGX, whose pin definitions are already in here and are different from the NX). but I'd sure expect that NVidia's github repo contains accurate data regarding NVidia's own hardware.
Closed
penguinland
added a commit
that referenced
this pull request
Feb 23, 2023
This is a follow-up to #1810. Thanks to Pete G for noticing the issue! We've been acquiring file descriptors for each GPIO pin, setting their direction to input/output, and if output, setting their value, and then closing the files again. It turns out that when you close the file, the value of the output is undetermined: the pin I tried last time (pin 40) keeps its most recent value, but pins like 37 and 32 float instead. So, this PR holds the file descriptors open for the remainder of the program (once they're opened for the first time). That way, the GPIO pins keep the same output all the way until the next time you change one of them.
randhid
pushed a commit
to randhid/rdk
that referenced
this pull request
Feb 25, 2023
Tried on a Jetson Orin: both reading and writing both high and low values work great! Also tried on a BeagleBone AI-64: GPIO behavior is unchanged. As Nicolas notes, it sucks to have 3 different implementations of GPIO stuff, and it would be wonderful to get down to a single approach that works everywhere. It's obvious why the pigpio approach doesn't work on a non-rpi board, and we've discussed why the periph.io approach doesn't work on the Orin. It would be neat to use the ioctl approach everywhere eventually, but that doesn't need to happen immediately. I learned great things from https://blog.lxsang.me/post/id/33 about how to use ioctl, and from https://github.com/mkch/gpio for how to do this in Go. I don't like how much duplication there is between `ioctlPin.Set` and `ioctlPin.Get`, but couldn't see an easy way to remove it: abstracting out a helper function to, say, open the device actually makes the code longer because you need to check the return value for errors, and the values to construct a `gpioHandleRequest` object are different enough that a helper function feels contrived to me.
randhid
pushed a commit
to randhid/rdk
that referenced
this pull request
Feb 25, 2023
This is a follow-up to viamrobotics#1810. Data is taken from https://github.com/NVIDIA/jetson-gpio/blob/master/lib/python/Jetson/GPIO/gpio_pin_data.py#L53-L80. I can't test this out because the Jetson Orin NX hardware is not yet on the market (we have the higher-end Orin AGX, whose pin definitions are already in here and are different from the NX). but I'd sure expect that NVidia's github repo contains accurate data regarding NVidia's own hardware.
randhid
pushed a commit
to randhid/rdk
that referenced
this pull request
Feb 25, 2023
…s#1896) This is a follow-up to viamrobotics#1810. Thanks to Pete G for noticing the issue! We've been acquiring file descriptors for each GPIO pin, setting their direction to input/output, and if output, setting their value, and then closing the files again. It turns out that when you close the file, the value of the output is undetermined: the pin I tried last time (pin 40) keeps its most recent value, but pins like 37 and 32 float instead. So, this PR holds the file descriptors open for the remainder of the program (once they're opened for the first time). That way, the GPIO pins keep the same output all the way until the next time you change one of them.
biotinker
pushed a commit
to biotinker/robotcore
that referenced
this pull request
Mar 16, 2023
Tried on a Jetson Orin: both reading and writing both high and low values work great! Also tried on a BeagleBone AI-64: GPIO behavior is unchanged. As Nicolas notes, it sucks to have 3 different implementations of GPIO stuff, and it would be wonderful to get down to a single approach that works everywhere. It's obvious why the pigpio approach doesn't work on a non-rpi board, and we've discussed why the periph.io approach doesn't work on the Orin. It would be neat to use the ioctl approach everywhere eventually, but that doesn't need to happen immediately. I learned great things from https://blog.lxsang.me/post/id/33 about how to use ioctl, and from https://github.com/mkch/gpio for how to do this in Go. I don't like how much duplication there is between `ioctlPin.Set` and `ioctlPin.Get`, but couldn't see an easy way to remove it: abstracting out a helper function to, say, open the device actually makes the code longer because you need to check the return value for errors, and the values to construct a `gpioHandleRequest` object are different enough that a helper function feels contrived to me.
biotinker
pushed a commit
to biotinker/robotcore
that referenced
this pull request
Mar 16, 2023
This is a follow-up to viamrobotics#1810. Data is taken from https://github.com/NVIDIA/jetson-gpio/blob/master/lib/python/Jetson/GPIO/gpio_pin_data.py#L53-L80. I can't test this out because the Jetson Orin NX hardware is not yet on the market (we have the higher-end Orin AGX, whose pin definitions are already in here and are different from the NX). but I'd sure expect that NVidia's github repo contains accurate data regarding NVidia's own hardware.
biotinker
pushed a commit
to biotinker/robotcore
that referenced
this pull request
Mar 16, 2023
…s#1896) This is a follow-up to viamrobotics#1810. Thanks to Pete G for noticing the issue! We've been acquiring file descriptors for each GPIO pin, setting their direction to input/output, and if output, setting their value, and then closing the files again. It turns out that when you close the file, the value of the output is undetermined: the pin I tried last time (pin 40) keeps its most recent value, but pins like 37 and 32 float instead. So, this PR holds the file descriptors open for the remainder of the program (once they're opened for the first time). That way, the GPIO pins keep the same output all the way until the next time you change one of them.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tried on a Jetson Orin: both reading and writing both high and low values work great! Not yet tried on a rpi or BeagleBone, but I will do both of those before merging.
As Nicolas notes, it sucks to have 3 different implementations of GPIO stuff, and it would be wonderful to get down to a single approach that works everywhere. It's obvious why the pigpio approach doesn't work on a non-rpi board, and we've discussed why the periph.io approach doesn't work on the Orin. It would be neat to use the ioctl approach everywhere eventually, but that doesn't need to happen immediately.
I learned great things from https://blog.lxsang.me/post/id/33 about how to use ioctl, and from https://github.com/mkch/gpio for how to do this in Go.
I don't like how much duplication there is between
ioctlPin.SetandioctlPin.Get, but couldn't see an easy way to remove it: abstracting out a helper function to, say, open the device actually makes the code longer because you need to check the return value for errors, and the values to construct agpioHandleRequestobject are different enough that a helper function feels contrived to me.I'm tagging Rand to review this PR because she knows good things from the BeagleBone, but part of me says Rand is already very busy and I ought to tag Gautham instead, because he's planning to look at the Jetson boards in the near future.