Maker Pro
Maker Pro

Wireless trigger for LEDs and buzzers

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
Yes that does look more efficient. When I first started I tried an array of bits but it wouldn't compile.

So just to double check. If I wanted to shift in a variable called NRF_config, I would call the above function like this:

Void shiftout(NRF_config);

The function would then take the value of NRF_config and place it into bytevalue? Bytevalue has a scope limited to that function right?
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
Also, when you're making the LSB of bytevalue onto RA3 that won't mess with any other bits on PORTA right?
 

KrisBlueNZ

Sadly passed away in 2015
Nov 28, 2011
8,393
Joined
Nov 28, 2011
Messages
8,393
So just to double check. If I wanted to shift in a variable called NRF_config, I would call the above function like this:
Void shiftout(NRF_config);
The function would then take the value of NRF_config and place it into bytevalue? Bytevalue has a scope limited to that function right?
Right. bytevalue is the local variable (local to that function) that will contain the value passed to the function as the function parameter.
Also, when you're making the LSB of bytevalue onto RA3 that won't mess with any other bits on PORTA right?
Right. Those assignments to port bits will be translated into bcf and bsf instructions. Those instructions read the port register, modify one bit, and write it back.

The only case where this can affect other bits is if your code changes those bits between input and output modes via the TRISx register.

If you set up the TRISx register at the start of your program, and never change it, there will be no interference. See the data sheet documentation on the PORTx registers for more information.
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
I'm writing the updated code as per your coding suggestions now, it's going to make the entire thing so much compact and cleaner looking. One thing I've thought of is will there be any timing issues with putting a value on the MOSI pin then the very next instruction bringing the SPI CLK high. I will have to check the datasheet to see if there needs to be a minimum amount of time before that transition
 

KrisBlueNZ

Sadly passed away in 2015
Nov 28, 2011
8,393
Joined
Nov 28, 2011
Messages
8,393
That's good that you recognised that as a possible issue. It won't be a problem unless you're using a very fast PIC with a slow peripheral such as a CD4000-series device, but it's a good idea to check.
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
I'm using the internal oscillator inside the PIC, so about a 1us instruction clk
 

KrisBlueNZ

Sadly passed away in 2015
Nov 28, 2011
8,393
Joined
Nov 28, 2011
Messages
8,393
OK, so the time between one transition and the next will be about 1 µs.

The timings are given on page 49 of the nRF24L01 specification. The relevant parameter is Tdc, data to clock setup time, shown on the diagram on page 48.

This parameter is the minimum amount of time needed by the nRF24L01 from when the data is valid, before the rising edge of the SPI clock. And it's 2 ns, so you're safe by a factor of about 500.

Other timings in the table are similarly fast, so you won't have any problems interfacing to that device.
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
So now that I have, what I assume is working code so far, I need to work on the ISR. The NRF has an IRQ pin that goes low whenever the NRF receives valid data. I have connected this pin to the PIC's RB7 pin which I have configured for an interrupt on change so that when the NRF pulls its IRQ pin low, the PIC will execute it's ISR which will contain a similar loop that will extract the data.

I have modelled the ISR after the loop that KrisBlueNZ provided on the previous page:


void interrupt isr(void)
{
// service PORTB ISR
INTCONbits.RBIE=0;//clear interrupt on change flag, may move this to the end of the ISR
uint8_t buffer =0;
for (uint8_t bitn = 0; bitn < 7; ++bitn)
{
PORTAbits.RA2 = 0; // Bring SPI clock low
PORTAbits.RA2 = 1; // Bring SPI clock high
buffer = buffer & 0xFF; // take value on MISO pin and place it on buffer
buffer <<= 1; // Shift bits downwards in buffer
}
uint8_t segment = buffer; //this integer will be assigned to a 7 segment LED to display the value provided to it by the NRF's data
}

Have I gone about this the correct way? The only thing I'm not sure of is that since RB7 is an interrupt on change pin I can't seem to find in the datasheet when the NRF stops pulling the IRQ pin low, I just want to make sure that my code doesn't start to oscillate.
 

KrisBlueNZ

Sadly passed away in 2015
Nov 28, 2011
8,393
Joined
Nov 28, 2011
Messages
8,393
It's not normally a good idea to do anything time-consuming in an ISR. Also, any I/O accesses and variable accesses made by an ISR need to be carefully considered. What happens if the IRQ happens while your mainline code is in the middle of an SPI communication to that IC? Or a different SPI device that you might add in future? If the interrupt handler accesses the SPI signals, it will fail to communicate with the SPI device, and it will disrupt the mainline's communication as well.

A better way is to use bitflags ("semaphores") to allow your interrupt handler to signal to the mainline that there is work to be done. The mainline can run around in an "event loop", checking each semaphore in turn and calling a function to deal with the event if the semaphore is set.

In this case, you don't even need an interrupt handler, because the PIC hardware will keep the interrupt request flag (in PIR1 or wherever it is) set until cleared by software. So that's the semaphore you need, right there.

You should find out what the nRF does with the interrupt signal. Does it require data to be read from it before it releases the IRQ line back high again? Or does it just pulse it low briefly? If the former, you will have to perform the required SPI action before you can safely reset the interrupt request flag.

Another technique that's useful in real-time applications with multiple simultaneous inputs and outputs is queues, or circular buffers. This is an area of memory that's used as a FIFO (first in, first out) buffer for communication between two independent tasks. The data in the queue may be fully formed messages, status codes, command bytes, or whatever is appropriate. The queue is managed with two pointers: a "put" pointer and a "get" pointer, which point to where data will be added and removed, respectively. When either pointer passes the top of the memory area reserved for the queue, it is reset to the bottom of that area. When the pointers are equal there is no data in the queue. Google for more information.
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
In this case, you don't even need an interrupt handler, because the PIC hardware will keep the interrupt request flag (in PIR1 or wherever it is) set until cleared by software. So that's the semaphore you need, right there.

Do you mean I don't even need an ISR, just poll the bit flag in the appropriate register, or that I don't need to have a bit flag in the ISR? If I leave the ISR without clearing the interrupt flag won't I just start back at the beginning of the ISR?

That's a good idea about keeping the ISR short as possible, never thought about that.
 

KrisBlueNZ

Sadly passed away in 2015
Nov 28, 2011
8,393
Joined
Nov 28, 2011
Messages
8,393
You don't need an ISR at all.

You could use an ISR to detect the interrupt request flag, clear it, set a flag in a variable somewhere, then return, but there's no point. If you have code in the mainline that's testing a flag, it might as well test the interrupt request flag in PIR1 (or wherever). Once it detects that flag set, it has to clear it explicitly, then take the appropriate action.
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
Alright so here is the new main(void) with an if statement that polls the interrupt flag:

while(1)
{
if (INTCONbits.RBIF==1)// poll the int flag to see if the NRF has data that needs to be processed
{
PICin=0;//buffer used to load data from the NRF
for (uint8_t bitn = 0; bitn < 7; ++bitn)
{
PORTAbits.RA2 = 0;// Bring SPI clock low
PORTAbits.RA2 = 1;// Bring SPI clock high
PICin =| PORTAbits.RA5;// take value on MISO pin and place it on buffer
PICin <<= 1;//shift bits upwards into buffer
}
INTCONbits.RBIF=0;//clear interrupt flag
}
else
PORTB = PICin;//this puts the value of PICin onto PORTB which is connected to a 7 seg LED
}



Is the line:

PICin =| PORTAbits.RA5;

The proper way to load data from the MISO pin?
 

KrisBlueNZ

Sadly passed away in 2015
Nov 28, 2011
8,393
Joined
Nov 28, 2011
Messages
8,393
That looks like it might work, but there is no "=|" operator; I think you mean "|=". With that change, it should work.

With SPI, there's no distinction between reading a byte and writing a byte (unless the slave has separate read and write enables, and I've never heard of that). So when you're reading eight bits from it, it is also reading eight bits from you. I guess it has a command-oriented or message-oriented SPI with a NOP (no operation) code? In that case, you should make sure that MOSI transmits that code as you shift the data bits in.

Check the nRF's SPI documentation. You may need to request data from it before it will return any information on MISO. In that case, you need to send one or more command bytes to it, ignore the values you receive in return, then send the appropriate number of dummy bytes while storing the values that it returns. Sorry, I don't have time to read up on this now.

For bytewise SPI access I have a single function that accepts a byte value to transmit, and returns the byte value received, since by definition every clock pulse you generate clocks one bit in each direction. This function would read something like:
Code:
uint8_t spi_txrx_byte(uint8_t txdata)
{
    uint8_t bitn;
    for (bitn = 0; bitn < 7; ++bitn)
    {
        PORTAbits.RA3 = (txdata > 127); // Copy bit 7 of txdata to RA3 (MOSI)
        PORTAbits.RA2 = 0;              // Bring SPI clock low
        PORTAbits.RA2 = 1;              // Bring SPI clock high
        txdata <<= 1;                   // Shift bits up in txdata
        txdata |= PORTAbits.RA5;        // Shift MISO pin state into bit 0
    }//for bitn
    PORTAbits.RA2 = 0;                  // Return SPI clock low (for tidiness)
    PORTAbits.RA3 = 0;                  // Ditto for MOSI
    return txdata;                      // txdata doubles as rxdata
}//spi_txrx_byte

That code uses the txdata variable to hold the data being shifted out, and the data being shifted in. Saves a little bit of storage and time :)

BTW, use the [CODE] and [/CODE] tags around code listings to make them monospaced and prevent indentation from being clobbered.
 

KrisBlueNZ

Sadly passed away in 2015
Nov 28, 2011
8,393
Joined
Nov 28, 2011
Messages
8,393
I just noticed something in the PIC16F627A data sheet. On page 38 of revision G (DS40044G) there's a grey box that says: "Note: If a change on the I/O pin should occur when a read operation is being executed (start of the Q2 cycle), then the RBIF interrupt flag may not get set."

This warning is not worded clearly enough for me to say for sure what implications it might have. Does anyone have any suggestions?
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
Check the nRF's SPI documentation. You may need to request data from it before it will return any information on MISO. In that case, you need to send one or more command bytes to it, ignore the values you receive in return, then send the appropriate number of dummy bytes while storing the values that it returns. Sorry, I don't have time to read up on this now.

The NRF expects an instruction byte first, as you shift in the instruction byte it simultaneously shifts out it's status register. After that if I'm reading I shift in garbage while it shifts out what I want, if I want to write I send it data and ignore what it sends back.

For bytewise SPI access I have a single function that accepts a byte value to transmit, and returns the byte value received, since by definition every clock pulse you generate clocks one bit in each direction.

I don't think I will need to do any simultaneous reading/writing to the NRF, but I'll bookmark that code, it could come in very handy.

I just noticed something in the PIC16F627A data sheet. On page 38 of revision G (DS40044G) there's a grey box that says: "Note: If a change on the I/O pin should occur when a read operation is being executed (start of the Q2 cycle), then the RBIF interrupt flag may not get set."

Does that pertain to only PORTB or the entire uController? I'm going to have a look at it now. I may have to incorporate multiple buffers to reduce the chance of any PORTA/B reading messing with the RBIF
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
BTW, use the CODE and /CODE tags around code listings to make them monospaced and prevent indentation from being clobbered.
It's amazing how much the readability is improved with indentation
 

KrisBlueNZ

Sadly passed away in 2015
Nov 28, 2011
8,393
Joined
Nov 28, 2011
Messages
8,393
The NRF expects an instruction byte first, as you shift in the instruction byte it simultaneously shifts out it's status register. After that if I'm reading I shift in garbage while it shifts out what I want, if I want to write I send it data and ignore what it sends back.
OK, good.
I don't think I will need to do any simultaneous reading/writing to the NRF, but I'll bookmark that code, it could come in very handy.
The point is that you can use that function to either read, write, or read and write simultaneously.
To read a byte: newdata = spi_txrx_byte(0); // write 0x00 and read the simultaneous response
To write a byte: spi_txrx_byte(byte_to_write); // write data; ignore simultaneous response

It replaces two separate and limited functions.

Does that pertain to only PORTB or the entire uController?
Exactly! That's why I said it was not clear.
I'm going to have a look at it now. I may have to incorporate multiple buffers to reduce the chance of any PORTA/B reading messing with the RBIF
How does the nRF drive the IRQ signal? Does it hold it active until it's acknowledged? If so, you don't need the RBIF anyway - just read the PORTB data register instead of polling the interrupt flag.
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
So if I wanted to transmit I would call it like this:

spi_txrx_byte(txdata);

and if I wanted to receive data I would call it like this?

receive_data = spi_txrx_byte(txdata);
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
I just noticed something in the PIC16F627A data sheet. On page 38 of revision G (DS40044G) there's a grey box that says: "Note: If a change on the I/O pin should occur when a read operation is being executed (start of the Q2 cycle), then the RBIF interrupt flag may not get set."
This warning is not worded clearly enough for me to say for sure what implications it might have. Does anyone have any suggestions?

P.44 of the PIC's datasheet clears things up a little I think. It says that any read of a pin reads the entire port
 

foTONICS

Sep 30, 2011
332
Joined
Sep 30, 2011
Messages
332
I mean I guess we could just poll that one pin repeatedly to see if the NRF has pulled it low and not worry about an interrupt at all
 
Top