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

Alter onewire libs so other non-ibutton pins can be used as well #1635

Closed
wants to merge 2 commits into from
Closed

Alter onewire libs so other non-ibutton pins can be used as well #1635

wants to merge 2 commits into from

Conversation

wdoekes
Copy link
Contributor

@wdoekes wdoekes commented Aug 22, 2022

What's new

Changes so onewire_host_alloc() now takes a mandatory GpioPin. For iButton, this would be &ibutton_gpio. For other uses, the &gpio_ext_pa7 (and other pins) can be used.

This changeset allows us to implement plugins/apps that speak the OneWire protocol over the top gpio pins.

Closes: #1596

Verification

  • Someone needs to confirm that the iButton functionality still works. I have no such thing to test.
  • To test the new behaviour, I hacked up a temperature sensor plugin where you can connect a DS18x20 temperature sensor and cycle through the pins using LEFT/RIGHT. Dirty version here: https://junk.devs.nu/2022/flipper/sensors_ds18x20_1.patch

image

  • In the image above, the DS sensor is connected to pin C0. The temperature reads 23.3 celcius.

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

@wdoekes wdoekes force-pushed the allow-onewire-libs-for-non-ibutton branch from 87f8a71 to 779f3c1 Compare August 22, 2022 22:30
DrZlo13
DrZlo13 requested changes Aug 23, 2022
View reviewed changes
firmware/targets/f7/furi_hal/furi_hal_ibutton.c Outdated Show resolved Hide resolved
firmware/targets/furi_hal_include/furi_hal_onewire.h
* Sets the pin to interrupt mode (EXTI interrupt on rise or fall), and sets it to float
* @param gpio
*/
void furi_hal_onewire_start_interrupt(const GpioPin* gpio);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please comment which EXTI (which pin) can be used as an interrupt.
V9 table from https://docs.google.com/spreadsheets/d/1qAPFTwvMwbTp1hTAtZHlO6jpl9o-fXg49rPIQ1o7JiQ/edit?usp=sharing
(0, 1, 4, 5, 7, 8, 14).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to add this, but I'm not sure what you mean. I'm not that initimate with electronics.

Are you saying all pins on A..H are interrupt-capable if they're on EXTI{0,1,4,5,7,8,14} and all others are not?

I.e.:

/*
 * Sets the pin to interrupt mode (EXTI interrupt on rise or fall), and sets it to float
 * The following pins are valid:
 * gpio_infrared_rx, gpio_display_rst_n, gpio_ext_pc0, gpio_subghz_cs (EXTI0),
 * gpio_cc1101_g0, gpio_display_di, gpio_ext_pc1, gpio_spi_d_sck (EXTI1),
 * ... etc ...

?

I have no idea what I'm typing here, so I could be off by a mile 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @DrZlo13 : any hint is appreciated :)

@hedger hedger added New Feature Contains an IMPLEMENTATION of a new feature Core+Services HAL, furi & core system services labels Aug 23, 2022
@wdoekes wdoekes force-pushed the allow-onewire-libs-for-non-ibutton branch from 64ab40c to bd1ab81 Compare August 24, 2022 08:57
@wdoekes
Alter onewire libs so other non-ibutton pins can be used as well
afc5a9d 
Changes so onewire_host_alloc() now takes a mandatory GpioPin. For
iButton, this would be &ibutton_gpio. For other uses, the &gpio_ext_pa7
(and other pins) can be used.

This changeset allows us to implement plugins/apps that speak the
OneWire protocol over the top gpio pins.

Closes: flipperdevices#1596
@wdoekes
Remove leftover furi_hal_ibutton_(start_drive|stop|pin_low)
e9a93c9 
Replace the calls with the new furi_hal_onewire_xx equivalents

Change request by @DrZlo13.
@wdoekes wdoekes force-pushed the allow-onewire-libs-for-non-ibutton branch from bd1ab81 to e9a93c9 Compare August 28, 2022 13:46
@wdoekes wdoekes mentioned this pull request Sep 20, 2022
Closed
3 tasks
@skotopes
Copy link
Member

skotopes commented Oct 8, 2022

Please feel free to reopen after fixing all requested changes.

@skotopes skotopes closed this Oct 8, 2022
@wdoekes
Copy link
Contributor Author

wdoekes commented Oct 8, 2022

Not fair @skotopes . I've asked for clarification, but not gotten any.

@wdoekes
Copy link
Contributor Author

wdoekes commented Mar 13, 2023

Looks like you added DS18B20 support in 7a3a1aa as well 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DrZlo13 DrZlo13 DrZlo13 requested changes

@skotopes skotopes Awaiting requested review from skotopes skotopes is a code owner

@hedger hedger Awaiting requested review from hedger hedger is a code owner

@gsurkov gsurkov Awaiting requested review from gsurkov gsurkov is a code owner

Assignees
No one assigned
Labels
Core+Services HAL, furi & core system services New Feature Contains an IMPLEMENTATION of a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

one_wire libs should not mandate calling furi_hal_ibutton_*
4 participants
@wdoekes @skotopes @DrZlo13 @hedger