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

Update ArduinoHal.h to make spi and friends protected #1044

Merged
merged 1 commit into from
Mar 31, 2024

Conversation

jp-bennett
Copy link
Contributor

Most of the "override" functions here can't actually be overridden in a useful way when spi, spiSettings, and everything else is marked private. If everything is override, then nothing should be private.

Most of the "override" functions here can't actually be overridden in a useful way when spi, spiSettings, and everything else is marked private. If everything is override, then nothing should be private.
@HeadBoffin
Copy link
Collaborator

What about the God Mode option?

@jp-bennett
Copy link
Contributor Author

What about the God Mode option?

Yes, that's how we'll manage this for now, but seems like a sub-par solution. Has too many side-effects, and isn't designed for a long term solution.

@jgromes
Copy link
Owner

jgromes commented Mar 31, 2024

I'm not sure I understand why this is necessary. Methods of ArduinoHal are not supposed to be overriden themselves, they are overriding the virtual methods from RadioLibHal, which is the base class of ArduinoHal. Though perhaps I'm missing something in your use-case.

@HeadBoffin
Copy link
Collaborator

What about the God Mode option?

Yes, that's how we'll manage this for now, but seems like a sub-par solution. Has too many side-effects, and isn't designed for a long term solution.

I mean't will God Mode continue to be available if this is merged?

@jp-bennett
Copy link
Contributor Author

jp-bennett commented Mar 31, 2024

I'm not sure I understand why this is necessary. Methods of ArduinoHal are not supposed to be overriden themselves, they are overriding the virtual methods from RadioLibHal, which is the base class of ArduinoHal. Though perhaps I'm missing something in your use-case.

We already use ArduinoHal as a parent class, and override spiBeginTransaction and spiEndTransaction to do some extra locking. It would be very useful to override spiTransfer to make use of the multi-byte transfer() methods available on some platforms.

Edit to clarify, this usage is from the Meshtastic project. The exact problem I'm seeing is that the single-byte writes bounces the chip select line for each write, when Linux itself is controlling the CS.

I mean't will God Mode continue to be available if this is merged?

Yes, should continue to work the exact same way. This change would just make these objects accessible for inherited classes, when god mode is turned off.

@jgromes jgromes merged commit e549361 into jgromes:master Mar 31, 2024
29 checks passed
@jgromes
Copy link
Owner

jgromes commented Mar 31, 2024

@jp-bennett thank you for the extra context, that makes the request quite clear. I think we can have the members protected to allow easier customization of the HAL.

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