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

Adds handling for arduino_default for specifying SPI device, and not CS #560

Closed
wants to merge 1 commit into from

Conversation

jp-bennett
Copy link
Contributor

@jp-bennett jp-bennett commented Apr 25, 2024

This is the so-simple-its-dumb approach to the SPI touchscreen issue. The idea is that we'd add SPI1 and SPI2 to portduino as extern defines, so they're always available. We can tie them to a real hardware device in our init code, and here we just use the spi_host int to specify which one to use. This is an alternative solution to #559

Edit: the addition of is_SPI is because now that we don't necessarily need a CS, there's no other way to determine for sure that it's an SPI device.

@jp-bennett
Copy link
Contributor Author

And obviously it's failing here, because the pre-requisite change isn't in Portduino.

@tobozo
Copy link
Collaborator

tobozo commented Apr 26, 2024

if your PR targets a specific version of meshtastic native platform you should as well update the platformio_native.ini file to point to the relevant commit hash so that the CI job doesn't fail

@@ -67,14 +67,15 @@ namespace lgfx
};
};
int16_t pin_cs = -1;
bool is_SPI = false;
Copy link

@mverch67 mverch67 Apr 26, 2024

Choose a reason for hiding this comment

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

I don't think this change at the interface is needed. The interface already has a way to specify if it's spi or i2c by querying the value of cs.
Rather than changing this interface the application that uses this interface has to be changed.

Choose a reason for hiding this comment

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

What about setting all four values (cs, mosi, miso, sclk) to 0 to indicate that this interface is managed by the Linux kernel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That might work. 0 is a valid pin number, but not all of them set to it. There is the problem of the union, which is in itself a really weird thing. So if we set the SPI elements to 0, it overwrites the I2C elements.

@jp-bennett
Copy link
Contributor Author

Closing this in favor of #561

@jp-bennett jp-bennett closed this Apr 28, 2024
@jp-bennett jp-bennett deleted the dumb-spi-fix branch April 28, 2024 18:25
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