Thanks, just proved it to myself. with this code - when the return type is bool it aways returns 1 irregardless which bit evaluated to true.
I really like to know how did the OP determined that the "results are different "?
Where is the evaluation of the returned value ?
Serial.begin(115200);
const int8_t REGISTER = 0x14;
const int8_t pin = 2;
bool result = REGISTER & (1<<pin) | 0x10;
if(result)
{
Serial.print("result ");
Serial.println( result,BIN);
while(1);
}
while(1);
Jiggy-Ninja:
Is this meant to emulate a N64 controller, or read signals from an N64 controller?Do you have a spec for the protocol?
I am working from this spec. The Arduino is acting as a Nintendo 64 console, sending queries to the controller and receiving its responses.
bool N64_Interface::query(char mask)
{
return PIND & mask; //this works too
}
So it appears that the difference between the code that works and the code that doesn't is a timing issue. Accessing the member variable in query() slows the code to the point where it doesn't return the right values, causing the watchdog timer in the receive() function to reset the Arduino.
I have used a static const within the class as a workaround, but the real goal is to have multiple objects each with its own pin.
LogicalUnit:
...but the real goal is to have multiple objects each with its own pin.
Then try what I suggested: remove / flatten query; store the mask in a local variable.
LogicalUnit:
I am working from this spec. The Arduino is acting as a Nintendo 64 console, sending queries to the controller and receiving its responses.bool N64_Interface::query(char mask)
{
return PIND & mask; //this works too
}
So it appears that the difference between the code that works and the code that doesn't is a timing issue. Accessing the member variable in query() slows the code to the point where it doesn't return the right values, causing the watchdog timer in the receive() function to reset the Arduino. I have used a static const within the class as a workaround, but the real goal is to have multiple objects each with its own pin.
In other words - it the bit is 0 your WDT fires?
Basically you are waiting for bit 2 to be set?
If so there is nothing wrong with query function - the issue is "down the line" .
That is why I asked how did you determine the query function is faulty, because it is not.
bool N64_Interface::query()
{
return (PIND & (1<<2)); //this works
}but this behaves differently:
bool N64_Interface::query()
{
return (PIND & (1<<data_pin_)); //where data_pin_ is a member integer equal to 2
}
The difference between these two statements is the result of compiler optimization.
When you put 1<<2 with 2 literal constants like that, the compiler can evaluate the shift statement and transform it to (PIND & 0x04), so the CPU doesn't have to calculate the shift each time, just the &. When you use a variable for the pin number, the compiler can't optimize that away and so the microcontroller has to calculate the bitshift every time it hits that part of the code.
I see from the changes in Github that you've tried out CB's idea to store the mask as the member variable instead of the bit number. How is that working?
Jiggy-Ninja:
The difference between these two statements is the result of compiler optimization.When you put 1<<2 with 2 literal constants like that, the compiler can evaluate the shift statement and transform it to (PIND & 0x04), so the CPU doesn't have to calculate the shift each time, just the &. When you use a variable for the pin number, the compiler can't optimize that away and so the microcontroller has to calculate the bitshift every time it hits that part of the code.
The same is true for const variables.
I'd be curious to know if the variable was declared as a const inside the class and assigned by the constructor using an initializer list, if it would behave like a simple constant the same way a static const variable in a class or a global const variable.
i.e. if you use a constant in the constructor and the constructor uses and initailzer to fill in the const value of the member in the object, is the compiler smart enough to be able to treat the value as constant at compile time.
This would probably also require puting the code that uses it in the class definition instead of in a separate .cpp file to give the compiler the needed visibility to do the optimization.
Should be fairly easy to write some test code to see how the compiler handles it.
--- bill
When you put 1<<2 with 2 literal constants like that, the compiler can evaluate the shift statement and transform it to (PIND & 0x04), so the CPU doesn't have to calculate the shift each time, just the &. When you use a variable for the pin number, the compiler can't optimize that away and so the microcontroller has to calculate the bitshift every time it hits that part of the code.
Gentlemen, with all respect to your academic knowledge - the issue is not if the process takes x cycles or y cycles , the issue is how are the results of the operation used.
If the application depends on speed of bit shifting , which the OP won't confirm, this discussion run its course IMHO.
Jim
julyjim:
Gentlemen, with all respect to your academic knowledge - the issue is not if the process takes x cycles or y cycles , the issue is how are the results of the operation used.
If the application depends on speed of bit shifting , which the OP won't confirm, this discussion run its course IMHO.
Jim
OP is doing timing manually with fixed numbers of NOPs, just about the worst possible way you could do this. The timing difference could very easily throw this off.
void N64_Interface::send(char const* input, unsigned int length) {
for (int i = 0; i < length; i++) {
char currentByte = input[i];
for (int j = 0; j < 8; j++) {
bool currentBit = currentByte & 0x80; //get leftmost bit
currentByte <<= 1; //shift left
if (currentBit) {
//send logical 1
low();
NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP;
high();
NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP;
NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP;
NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP;
} else {
//send logical 0
low();
NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP;
NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP;
NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP;
high();
NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP; NOP;
}
}
}
This should definitely be rewritten to use a timer interrupt.
What I am really trying to do here is wait for a falling edge on the data pin. This indicates the start of a bit. I have tried to remove the bit-shift by precalculating it when the data pin is specified. I have also tried storing this value in a const member. All I can tell is that the code works great when the data pin is stored in a local variable or static const, but not when the data pin is stored in a member variable.
Why did you say that in first place?
Here is a sample how to detect rising edge. For trailing just remove the !
// wait for TDRE
while (!(spi->SPI_SR & SPI_SR_TDRE))
{
// optional timeout here
}
When shifting signed values the results of left-shift are undefined in C/C++
Left shift is fine. It's right shift of signed numbers that is undefined...
return PIND & data_pin_mask_; // doesn't work, data_pin_mask_ = 0x04.
It certainly should work. the Serial code is full of this sort of thing. Probably you have some error with setting or defining data_pin_mask_, but no one can tell because you haven't posted that part of the code, despite repeated requests to post everything.
I have solved the problem. The solution was to create a local copy of the member variable inside the receive() function, and then reduce the number of cycles to wait before sampling the line.
Thanks to all who contributed.