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.

Sign up for GitHub

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

Jump to bottom

Return {error, Reason} from gpio operations in error conditions #894

Open
fadushin opened this issue Oct 25, 2023 · 3 comments · May be fixed by #1408
Open

Return {error, Reason} from gpio operations in error conditions #894

fadushin opened this issue Oct 25, 2023 · 3 comments · May be fixed by #1408
Labels
breaking This change has the potential to break existing applications lib:eavmlib Erlang AtomVM library

Comments

@fadushin
Copy link
Collaborator

The AtomVM gpio interface returns bare error atoms in error conditions, which can make it difficult to diagnose why an error occurred.

We need to change the return type of these operations from error to {error, Reason :: term()}, where Reason is an appropriate term that can be used for logging or even for a program to take action.

The following functions need to be modified:

  • gpio:close/0
  • gpio:stop/0
  • gpio:set_direction/3
  • gpio:set_level/3
  • gpio:set_int/3
  • gpio:remove_int/2
  • gpio:set_pin_mode/2
  • gpio:set_pull_mode/2
  • gpio:hold_en/1
  • gpio:hold_dis/1
  • gpio:digital_write/2
  • gpio:attach_interrupt/2
  • gpio:detach_interrupt/1
@fadushin fadushin added lib:eavmlib Erlang AtomVM library breaking This change has the potential to break existing applications labels Oct 25, 2023
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Oct 25, 2023
@UncleGrumpy
Improved stm32 GPIO driver
2a025b6 
Complete rewrite of the stm32 gpio driver using updated APIs.

Moves all GPIO related atoms from `platform_defaultatoms.{c,h}` into `gpio_driver.c`
Improved input validation.
Adds read support port driver.
Adds nif functions to support read and write.
Adds cmake configuration options to disable nifs and/or port driver when building the VM.

Closes atomvm#571
Addresses atomvm#894 on STM32 platform.

Signed-off-by: Winford <[email protected]>
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Oct 25, 2023
@UncleGrumpy
Improved stm32 GPIO driver
59fb054 
Complete rewrite of the stm32 gpio driver using updated APIs.

Moves all GPIO related atoms from `platform_defaultatoms.{c,h}` into `gpio_driver.c`
Improved input validation.
Adds read support port driver.
Adds nif functions to support read and write.
Adds cmake configuration options to disable nifs and/or port driver when building the VM.

Closes atomvm#571
Addresses atomvm#894 on STM32 platform.

Signed-off-by: Winford <[email protected]>
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Oct 25, 2023
@UncleGrumpy
Improved stm32 GPIO driver
cff46fc 
Complete rewrite of the stm32 gpio driver using updated APIs.

Moves all GPIO related atoms from `platform_defaultatoms.{c,h}` into `gpio_driver.c`
Improved input validation.
Adds read support port driver.
Adds nif functions to support read and write.
Adds cmake configuration options to disable nifs and/or port driver when building the VM.

Closes atomvm#571
Addresses atomvm#894 on STM32 platform.

Signed-off-by: Winford <[email protected]>
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Oct 26, 2023
@UncleGrumpy
Improved stm32 GPIO driver
5e26434 
Complete rewrite of the stm32 gpio driver using updated APIs.

Moves all GPIO related atoms from `platform_defaultatoms.{c,h}` into `gpio_driver.c`
Improved input validation.
Adds read support port driver.
Adds nif functions to support read and write.
Adds cmake configuration options to disable nifs and/or port driver when building the VM.

Closes atomvm#571
Addresses atomvm#894 on STM32 platform.

Signed-off-by: Winford <[email protected]>
@UncleGrumpy UncleGrumpy mentioned this issue Oct 26, 2023
Merged
@fadushin fadushin mentioned this issue Oct 26, 2023
Merged
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Oct 26, 2023
@UncleGrumpy
Update rp2040 gpio driver error returns
90d792b 
Update error returns from the gpio driver to return {error, Reason} rather than
just an uninformative `error`.

This will complete rp2040 platform requiremments to satisfy the ongoing meta
issue atomvm#894.

Signed-off-by: Winford <[email protected]>
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Oct 26, 2023
@UncleGrumpy
Update rp2040 gpio driver error returns
90b5290 
Update error returns from the gpio driver to return {error, Reason} rather than
just an uninformative `error`.

This will complete rp2040 platform requiremments to satisfy the ongoing meta
issue atomvm#894.

Signed-off-by: Winford <[email protected]>
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Oct 26, 2023
@UncleGrumpy
Update rp2040 gpio driver error returns
fdffe16 
Update error returns from the gpio driver to return {error, Reason} rather than
just an uninformative `error`.

This will complete rp2040 platform requiremments to satisfy the ongoing meta
issue atomvm#894.

Signed-off-by: Winford <[email protected]>
@pguyot
Copy link
Collaborator

pguyot commented Oct 27, 2023

We definitely need to align semantics of gpio module across platforms.
We may want to fail with badargs when arguments are inappropriate instead of returning error atom as we currently do.

@fadushin
Copy link
Collaborator Author

I agree on badarg errors as well. Let’s take a look at patterns in OTP for guidance on expected behavior.

@UncleGrumpy
Copy link
Collaborator

In almost all cases badarg does make sense. With some exceptions, gpio:stop/0 and gpio:close/1 should probably RAISE_ERROR(NOPROC_ATOM) if no gpio driver process is currently running, that might also be the most appropriate response to attempting to remove an interrupt that isn't set. I'm not sure what the return for setting an interrupt on a pin that is already attached should be, the argument is still valid.

@fadushin fadushin mentioned this issue Dec 8, 2023
Merged
@DawDavis DawDavis mentioned this issue Mar 23, 2024
Open
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Dec 21, 2024
@UncleGrumpy
Fix ESP32 gpio driver error returns to match spec
cf842e6 
Changes error returns to match the spec and ofther platforms. Nifs now raise any errors, and port
function errors return `{error, Reason}`.

Closes atomvm#894

Signed-off-by: Winford <[email protected]>
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Dec 21, 2024
@UncleGrumpy
Fix ESP32 gpio driver error returns to match spec
587b4d4 
Changes error returns to match the spec and ofther platforms. Nifs now raise any errors, and port
function errors return `{error, Reason}`.

Closes atomvm#894

Signed-off-by: Winford <[email protected]>
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Dec 21, 2024
@UncleGrumpy
Fix ESP32 gpio driver error returns to match spec
aca3489 
Changes error returns to match the spec and ofther platforms. Nifs now raise any errors, and port
function errors return `{error, Reason}`.

Closes atomvm#894

Signed-off-by: Winford <[email protected]>
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Dec 21, 2024
@UncleGrumpy
Fix ESP32 gpio driver error returns to match spec
fa94ff3 
Changes error returns to match the spec and ofther platforms. Nifs now raise any errors, and port
function errors return `{error, Reason}`.

Updates gpio.erl to reflect the correct returns, and make it more clear that errors for nifs will be
raised.

Closes atomvm#894

Signed-off-by: Winford <[email protected]>
@UncleGrumpy UncleGrumpy linked a pull request Dec 21, 2024 that will close this issue
Open
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Dec 22, 2024
@UncleGrumpy
Fix ESP32 gpio driver error returns to match spec
c4c5343 
Changes error returns to match the spec and ofther platforms. Nifs now raise any errors, and port
function errors return `{error, Reason}`.

Updates gpio.erl to reflect the correct returns, and make it more clear that errors for nifs will be
raised.

Closes atomvm#894

Signed-off-by: Winford <[email protected]>
UncleGrumpy added a commit to UncleGrumpy/AtomVM that referenced this issue Dec 22, 2024
@UncleGrumpy
Fix ESP32 gpio driver error returns to match spec
24fdbf9 
Changes error returns to match the spec and ofther platforms. Nifs now raise any errors, and port
function errors return `{error, Reason}`.

Updates gpio.erl to reflect the correct returns, and make it more clear that errors for nifs will be
raised.

Closes atomvm#894

Signed-off-by: Winford <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking This change has the potential to break existing applications lib:eavmlib Erlang AtomVM library
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Updates to ESP32 GPIO driver to match other MCU platforms UncleGrumpy/AtomVM
3 participants
@pguyot @fadushin @UncleGrumpy