Strange behavior from my code. PIC16F1826 inside

[Pigi_102]

Hi all.
I' m wirting a code to decode a signal from a source.

The source sends out a bit train in this fashion: 10100000 10100001 10100010 10100011

In my code I read this data train and write it in hex in the internal eeprom.

I wrote the routine and checkd in mplab using some test code in this way:

---

Code C - [expand]
1
2
3
4
5
strcpy(pckt_bits,(char *)"10100000");
strcat(pckt_bits,(char *)"10100001");
strcat(pckt_bits,(char *)"10100010");
strcat(pckt_bits,(char *)"10100011");
data = get_data();

---

just outside the while loop.

In the get_data

I write to eeprom the bit received and the computed value:

---

Code C - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
int8 get_data(int8 start_address)
{
  int8 ret = 0;
  int8 i;
  for (i = (start_address); i < (start_address) + 8; i++) {
      ret <<= 1;
      if(pckt_bits[i] == '1')
         ret |= 1;
         write_eeprom(address,pckt_bits[i]);
         address++;
  }
  write_eeprom(address,ret);
  address++;
  write_eeprom(address,254);  // Stop indicator
  address++;
  return ret;
}

---

When I read the eeprom value outside the while loop ( tr previous strcpy/strcat part )I read 01 00 01 00 00 00 00 00 A0 FE and it's exactly what I do expect.

The main loop is:

---

Code C - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
while(1){
      
      for(pckt_bits_pos=0; pckt_bits_pos < PACKET_BITS ; pckt_bits_pos++){  
          while (input(CLK));   // Wait for clock low
          pckt_bits[pckt_bits_pos] = input_state(DI);
          while (!input(CLK));          // wait for clock high
      }
      data= get_data();
      for (pckt_bits_pos = 0; pckt_bits_pos < PACKET_BITS; pckt_bits_pos++) {
          pckt_bits[pckt_bits_pos] = 0;
      }
}

---

When the program is in the while loop in the eeprom I read: 01 00 01 00 00 00 00 00 00 FE

So the last byte seem that is has not get computed.


The are no interrupt in the code, but hell if I can understand why it's working this way.


The PIC is a 16F1826,

---

Code C - [expand]
1
2
3
4
5
6
7
8
#FUSES NOBROWNOUT               //No brownout reset
#FUSES INTRC_IO                 //Internal Oscillator
#FUSES NOWDT                    //No Watch Dog Timer
#FUSES NOPUT                    //No Power Up Timer
#FUSES NOPROTECT                //Code not protected from reading
#FUSES MCLR                     //Master Clear pin enabled
#FUSES NOLVP                    //Low Voltage Programming on B3(PIC16) or B5(PIC18)
#FUSES NOCPD                    //No EE protection

---

The compiler is CCS PCWHD 5.008

Do you have any idea ?

Thanks

Pigi

 

[xenos]

Wrong code in your loop:

pckt_bits[pckt_bits_pos] = input_state(DI);

Corrected code:

pckt_bits[pckt_bits_pos] = input_state(DI) + '0';
//or
//pckt_bits[pckt_bits_pos] = input_state(DI)?'1':'0';

The integer value 0 is not equal to the ascii value '0'

ps: It's a time and memory consuming approach what you do. I think you should reconsider your algorithm.

 

[Pigi_102]

ps: It's a time and memory consuming approach what you do. I think you should reconsider your algorithm.

Can you explain better, pleas xenos ?

I know it's inefficent, but wich part you are referring to ?

Thanks

Pierluigi

 

[xenos]

When you get 1 ascii character from the UART, it is eighter 1 or 0. Why you spend 1 byte to store in memory and 1 byte to store in eeprom plus the appropriate time? The eeprom writing time is relatevly high and the lifetime duration of eeproms (write cycles) is not unlimited (like RAMs)

Read 8 bits from UART and store to 1 byte directly.
eg.

      short int i;
      unsigned char res = 0;
      for(i=0; i<8 ; i++){  
          while (input(CLK));   // Wait for clock low
          res <<= 1;
          res |= (input_state(DI) & 1);
          while (!input(CLK));          // wait for clock high
      }
      return res;

 

[Pigi_102]

This is very interessant, and I whould think at it if I can adopt in my code.
I'm not reading from an uart, but from the tacho signal of a mill.
The protocol is based on 4 packet of 17 bits, controlled by a clock ( a kind of Spi, but different in some way ).
You can read about this protocol here:
https://macpod.net/misc/sx2_tachometer/sx2_tachometer.php
almost at half page.

Being that I have to read a single bit ( DATA ) when CLOCK is falling ( and CS is low ).

That's why i took the approach of read a single bit.

I'm open to everything that could help me to make a better code

Thanks for now, as my first prototype is working well, thank to your suggestions.

Pigi

 

[KlausST]

Hi,

What about giving the clock to a pin with the ability to raise an interrupt.
Within the ISR read the data pin and process it..

Klaus

 

[Pigi_102]

Hi,

What about giving the clock to a pin with the ability to raise an interrupt.
Within the ISR read the data pin and process it..

Klaus

I have tried that, but it seems that PIC are not so efficent in handling the interrupts.
They take a lot of cpu cycle and the timing are very short.
I've computed that with a 16F628 (at 20Mhz) I was missing one or two clock cycle every interrupt.

IIRC the interrupts in PIC "costs" 4 cpu cycle.

Pigi

 

[KlausST]

Hi,

They take a lot of cpu cycle

it should be less than in a busy wait loop.

one or two clock cycles every interrupt. Maybe you did heavy calcualtions within your ISR?

Klaus

 

[Pigi_102]

it should be less than in a busy wait loop.

I have been told, in other sites, that an interrupt routin, in CCS and at 4Mhz with a pic16F628 costs 56us, the clock frequency is 8us.
Also moving to 20Mhz I could loose some clock. Am I right ?

In a while loop I should loose less time, or at least with a while loop ( and a 16F1826 at 32Mhz ) I get all the

one or two clock cycles every interrupt. Maybe you did heavy calcualtions within your ISR?

Klaus

No calculations. Only the acquisition of the pin.

Pigi

 

[xenos]

Have you consider the eeprom write time compared to the CLOCK duration? It's quite long.
What you will do with the byte that you just read?
How much time have you got between byte transmission?

I do not think that it can be done if clock is too fast.
A PIC18FXXXX at 40MHz or above could help you on this.
If you insist on PIC16, you could consider other options.

Basically the 8usec time (not frequency) of the up clock, means 16usec period.
You could also find an other way to retrieve data by hardware, using a serial to parallel IC with latched output that will load 8 bits serialy while maintaining the previous byte read in the latched output. In this case the PIC will load in a ISR the byte and will have enough time to spend in other jobs.

- - - Updated - - -

I have been told, in other sites, that an interrupt routin, in CCS and at 4Mhz with a pic16F628 costs 56us, the clock frequency is 8us.

seems a lot of time. I never used C on PIC16 processors, but in assembly it's quite shorter.

Also moving to 20Mhz I could loose some clock. Am I right ?

Faster speed, faster response, more time to spend around. Faster is better for execution of heavy procedures. Faster is more power consuming.

 

[KlausST]

Hi,

The interface has a clock and a data line, so it is similar to SPI. Why can't you use SPI periferal for this?

The 8us you are talking about, is it the clock period time of the interface? (Rising edge to rising edge)
Or what is the interface clock frequency?

Klaus

 

[Pigi_102]

it should be less than in a busy wait loop.

I have been told, in other sites, that an interrupt routin, in CCS and at 4Mhz with a pic16F628 costs 56us, the clock frequency is 8us.
Also moving to 20Mhz I could loose some clock. Am I right ?

In a while loop I should loose less time, or at least with a while loop ( and a 16F1826 at 32Mhz ) I get all the

one or two clock cycles every interrupt. Maybe you did heavy calcualtions within your ISR?

Klaus

No calculations. Only the acquisition of the pin.

Pigi

 

[xenos]

...
No calculations. Only the acquisition of the pin.

were and how, did you try to store the bits during ISR?

 

[Pigi_102]

---

Code C - [expand]
1
2
packet_bits[packet_bits_pos] = input_state(DI);
  packet_bits_pos++;

---

This was the content of ISR ( also if it should have been input_state(DI)+'0')

No big caclulation I suppose ( also if this, translated from C to asm could have been more than a small calculation )

Pigi

 

[xenos]

---

Code C - [expand]
1
2
packet_bits[packet_bits_pos] = input_state(DI);
  packet_bits_pos++;

---

This was the content of ISR ( also if it should have been input_state(DI)+'0')

No big caclulation I suppose ( also if this, translated from C to asm could have been more than a small calculation )

Pigi

Even if you stored in eeprom outside the ISR, the previous saving to eeprom will not be completed before the next ISR occur.
I cannot even understand why you do the acquisition, without processing the data or even display the data. How can you know if you have data correctly received? Only by simulation?
Maybe you should tell what you want to do so you will get specific proposals.

 

[Pigi_102]

The storing in eeprom was only for debug purpose.
Now I have remove it completely.

The code showed was only to understand why it was bheaving different in two phases of program, but xenos got the right solution.
In the debug part I was using char, while in the main loop I was using int.
In the check part I was checking toward char, thus the right solution was to add "0" in the acquisiton phase.

Thanks for all the support, now it's working ( and already in production )

Pigi