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

Scoped SPI transactions #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

crteensy
Copy link
Contributor

Added SPI_TRANSACTION_BLOCK macro

added SPI_TRANSACTION_BLOCK macro
@PaulStoffregen
Copy link
Owner

I sent a message to Arduino's developer mailing list, asking for feedback and whether they might like to ever incorporate this into Arduino's official SPI library. Before I do anything, I'd like to allow at least a week for conversation there.

If this ends up forgotten without any activity (and without massive controversy from Arduino), please ping me in early November. That still should be plenty of time to get merged, for Teensyduino 1.21 in January.

@crteensy
Copy link
Contributor Author

Noted, "ping Paul in early november."

In the meantime, we might come up with a clever solution for the nested loop problem I pointed out in the forum...it breaks certain program flows.

@crteensy crteensy closed this Oct 12, 2014
@crteensy crteensy reopened this Oct 12, 2014
@crteensy
Copy link
Contributor Author

I suck at using git.

@matthijskooijman
Copy link

Proposed syntax looks good and useful to me.

I didn't really like that done variable at first, but looking closer it seems like a good way to make sure the loop runs exactly once. I am wondering if it's really necessary to put it inside the struct and even more if you need a done() accessor - just extra clutter.

Did you look at the produced machine code? I'm wondering if the compiler is smart about this, or if it introduces an actual loop and done variable...

@crteensy
Copy link
Contributor Author

@matthijskooijman To be honest I didn't look at the produced code, and with some minor trickery it might even be possible to let gcc remove all the unnecessary loop-ness although it doesn't unroll loops by default. That would have to enabled with the -funroll-loops option, which is apparently not included in the usual optimization options.

More important might be the fact that the proposed macro breaks the "expected" behavior (the behavior expected by those who just "use" the macro) of break and continue statements if it is used within an additional outer loop; see http://forum.pjrc.com/threads/26808-Scoped-SPI-transactions?p=56113&viewfull=1#post56113

@matthijskooijman
Copy link

Agreed on the break problem. I can't really see a way around this, though. You need to introduce a variable into the next block scope, and the only way I can think of right now is using a for loop. The ATOMIC_BLOCK macro suffers from the same problem, which is an indication that no solution exists for this (at least not in C, ATOMIC_BLOCK can't use C++).

@crteensy
Copy link
Contributor Author

The cleanest solution is the introduction of a guard class that manages an SPI transaction (construct: beginTransaction(), destruct: endTransaction()). However, nothing keeps users from actually storing such a guard object, in which case it might block the SPI forever. That's why I tried the anonymous struct trick.
One way to make this a bit safer, though, would be a mandatory constructor argument of type struct I_PROMISE_NOT_TO_STORE_THIS {}; or similar.

@matthijskooijman
Copy link

Nothing wrong with having an anymous struct like this (did I say something to suggest that?).

Also, users are free to call beginTransaction and never endTransaction already, of course :-p

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