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.

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

added driver for ads130e08 adc #462

Merged
merged 12 commits into from
Oct 23, 2022
Merged

added driver for ads130e08 adc #462

merged 12 commits into from
Oct 23, 2022

Conversation

weslleymfd
Copy link
Contributor

Hello there,

Here is a driver that I developed for ESP32 to work with the ADS130E08 ADC from Texas Instruments.

@@ -0,0 +1,689 @@
/*
* The MIT License (MIT)
Copy link
Collaborator

Choose a reason for hiding this comment

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

the source code says it's MIT, but the license file says otherwise. which is the one you intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MIT ... fixed on commit #b4e1988


CHECK(read_reg_8(dev, ADS130E08_REG_CONFIG1, &config1));

config->clk_en = (config1 & CONFIG1_MASK_BITS_CLK_EN);
Copy link
Collaborator

Choose a reason for hiding this comment

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

config might be NULL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed on commit #b3f0e37


## What it does

The example configures one `ADS130E08` device on a `SPI` bus.
Copy link
Collaborator

Choose a reason for hiding this comment

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

would you elaborate a bit more? logs from a successful test would be also nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll add it shortly.

depends:
thread_safe: yes
targets:
- name: esp32
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you have ESP32C3 or other chips to test? with changes to the pin assignment, it should work. if you don't, something like "should work on other ESP32" in the doc would be nice.

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 don't have other ESP32 chips to test. I'll change the doc and say it may be supported for other chips.

@weslleymfd
Copy link
Contributor Author

@trombik , I made the improvements

@trombik
Copy link
Collaborator

trombik commented Oct 21, 2022

@weslleymfd I am bit busy today and the weekend. but @UncleRus would comment if he has any (and I'm really happy to see his commit again!)

@UncleRus
Copy link
Owner

@weslleymfd @trombik Thank you!

@UncleRus UncleRus merged commit bc66344 into UncleRus:master Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants