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

Bug: mpr121 pin setup incorrect #18

Open
dromer opened this issue Dec 18, 2023 · 3 comments
Open

Bug: mpr121 pin setup incorrect #18

dromer opened this issue Dec 18, 2023 · 3 comments

Comments

@dromer
Copy link
Contributor

dromer commented Dec 18, 2023

When using a setup like:

	"parents": {
        "mpr121_driver": {
			"component": "Mpr121",
			"pin": {
			  "scl": 11,
			  "sda": 12
			}
        }
    },

We get compile errors like:

HeavyDaisy_Untitled.hpp:63:45: error: no match for 'operator=' (operand types are 'daisy::Pin' and 'dsy_gpio_pin')
   63 |     mpr121_driver_config.scl = som.GetPin(11);
      |                                             ^

Coming from this line in the component defs: https://github.com/electro-smith/json2daisy/blob/main/src/json2daisy/resources/component_defs.json#L558

I wonder if it would make sense to have the i2c setup for the mpr121 sensors more like the other i2c peripherals and delegate this to an i2c parent config.

@dromer
Copy link
Contributor Author

dromer commented Dec 23, 2023

So this is apparently because of the migration to Pin config and phasing out of dsy_gpi_pin and GetPin() functionality.

It would be good to make sure that json2daisy is all set for this deprecation as right now the use of GetPin() is quite wide-spread in the header template and component_defs.json.

For instance: https://github.com/electro-smith/json2daisy/blob/main/src/json2daisy/templates/daisy.h#L221-L225

@stephenhensley
Copy link
Contributor

@dromer thanks for the report.
I agree, I think migrating everything to use the modern Pin config sooner than later is important.

I'll try to slot this into our team's schedule in the next few weeks.
This should be almost, if not entirely, and under-the-hood refactor, and I don't think would require any changes to existing json config files.

@dromer
Copy link
Contributor Author

dromer commented Jan 3, 2024

Well I don't think until now many people have been using mpr121 with json2daisy. It was because @jonwatersloot and others started poking at it because of the SynthUX challenge that we found out the limitations with this implementation ;)

[edit: actually I'm wrong, earlier last year there was the report on the forum about this, but I only found this after we ran into these issues]

Being able to use an i2c bus for multiple simultaneous peripherals should definitely be possible at least imo.

(although I was actually able to use both an mpr121 and an i2c oled on the same pins without much trouble)

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

No branches or pull requests

2 participants