I'll eat my hat if...

Go To Last Post
34 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'll eat my hat if someone could tell me why this segment of code does not set pin PB22 high.  AVR32-0512.

 

else if((icmp_header->type[0] == ICMP_REP) || (icmp_header->type[0] == ICMP_UNREACH)){
   
​	handle_ping_response(ip_header);
   	
	gpio_set_pin_high(AVR32_PIN_PB22);respond_to_ping(ethernet_header);
​}
  

​It appears not to execute handle_ping_response or switch the led on but I inserted a piece of code called respond_to_ping and it executes?  Strange.  I would love to wrap this code up tonight but I fear I'll but dumb founded for days.

 

Thanks guys.

Last Edited: Wed. Oct 11, 2017 - 12:10 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Step through the code! Check the values. That normally enlightens me as to why things don’t execute the way i expect.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I actually tried debugging the code, (I never use the debugger) and it wont let me place a breakpoint any where near it.  I'll give it a try again the morrow.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:
it wont let me place a breakpoint any where near it

Then make sure code is actually generated.

 

E.g. if something makes the code unreachable then the compiler is very likely to not generate code at all. What could make that code unreachable? E.g. earlier condition in your nested if-statement is behaving like a "catch-all".

 

(How close do I need to be in my guesses for you to start cooking the hat? ;-)

 

Re the respond_to_ping(), do you have other calls to that in other places?

 

Fianawarrior wrote:
I never use the debugger

Why not?

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

With complex tests and arguments like those, I find it useful to set some variable to be a portion of that argument. For example, somewhere before that if-test

debug0 = icmp_header->type[0];

It may well be that you are not even executing that block of code! Checking debug0 and whether or not that branch of the if-test is executed should tell you a lot.

 

Jim

Jim Wagner Oregon Research Electronics, Consulting Div. Tangent, OR, USA http://www.orelectronics.net

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Interesting comments, it's 5:30am here in Ireland and I'm still up ;)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'd add that posting such a small section of your code makes it almost impossible for anybody to understand what's going on ( just so you know, for any future code postings ). Did you check the generated *.asm to see if the function even got compiled ?

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

Last Edited: Wed. Oct 11, 2017 - 05:00 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

tried everything to shed light on this but nothing gives.  have a look at the code below, it should execute!

 

 volatile struct ICMP_HEADER * icmp_header = (struct ICMP_HEADER *)&ip_header->data;
 
 icmp_header->type[0] = ICMP_REP;

 if((icmp_header->type[0] == ICMP_REP) || (icmp_header->type[0] == ICMP_UNREACH)){
  	gpio_set_pin_high(AVR32_PIN_PB22);_handle_ping_response(ip_header);
 }

 

 

 

Last Edited: Wed. Oct 11, 2017 - 04:51 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Found it, the compiler was trying to be smart with a if, else if list.  bed time.  night night freaks!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

As others have said, you are not showing enough code.

 

Are you sure you're even getting to

 

 volatile struct ICMP_HEADER * icmp_header = (struct ICMP_HEADER *)&ip_header->data;

?

 

You might be fooling yourself by thinking the if-statement must "catch" since you've forced it with

 

 icmp_header->type[0] = ICMP_REP;

but if you don't even get near that "forced conditional"...

 


 

Here are lessons I learned again and again (getting a little better at it each time) over the cause of software development for 35+ years:

 

Whenever things seems really strange it is time to take a step back and look for a natural explanation "outside my current box of thinking".

 


 

Move the lighting of the LED back (i.e. earlier) in the execution to where it lights up. Move it forward again to verify it does not light up. That "grapple" is an interesting point to study!

 

Get used to study generated assembler at the points which seems to be "troublesome". (Not that it is always the case, but being unable to place a breakpoint "any way near it" is just creaming at you to look at the disassembly.

 

And first and foremost: Get used to using an on-chip debugger! It makes for a much more effective debugging experience than "tracing with a LED". Faster. More rich on information.

 

There are of-course cases when on-chip debugging might mess things up by e.g. suggesting a bug while it actually is a "false positive". This is important to realize. Example: You have a stream of data coming into your controller. If you stop at a breakpoint then you risk data overrun and/or loss of data that might later look like a bug in your code.

 

LED debugging is simple to implement, but poor on "information richness".

 

UART logging/debugging is harder to implement, but can be richer on information. Of-course, if your UART(s) is(are) already occupied then this is not the way.

 

Real-hardware on-chip debugging is simple to implement, and very rich on information. There is a learning curve before it starts to really pay off but IMO it is definitively worth it!

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Wed. Oct 11, 2017 - 06:41 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I’m debugging some encrypted protocol stuff at the moment - rather than do it on the target, i’m doing it on the PC (mac actually). This is saving me a whole heap of time. I’m debugging both ends at the one time - i’ve set up a test harness that has both instances and rather than send packets via serial, it just uses a mock function that copies the data to the other instance. Since you’re just doing tcp/ip, there’s no realtime constraint and you can send your packets via a virtual ethernet interface to the real world. No need to use Windows either.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Fianawarrior wrote:
(I never use the debugger)
Curious statement from someone programming silicon as complex as AVR32. It's a bit like driving a car blindfold!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:
(I never use the debugger)
 

surprise

JohanEkdahl wrote:
Why not?

+99

clawson wrote:
Curious statement from someone programming silicon as complex as AVR32. It's a bit like driving a car blindfold!

Indeed - or like tying one hand behind your back.

Or, as JS put it:

js wrote:
[not having a debugger is] like a mechanic not having any spanners.

 

http://www.avrfreaks.net/comment...

 

 

EDIT

 

Johan's quote

Last Edited: Wed. Oct 11, 2017 - 08:53 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks guys for the advise, I learned a lot last night, namely that coffee and cigs help debugging. ;)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Coffee is possibly good for you

cigs may or will death you; consider vaping or ideally quitting (the only use for cigs is as a stimulant as each dose gets you one hour)

Avoid soft drinks

May you sleep/nap well and become a grey-haired (live long and prosper) debugger wink

 

"Dare to be naïve." - Buckminster Fuller

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

lol, the craic!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

gchapman wrote:

cigs may or will death you;

 

But what about eating hats then? :-)

 

-- Thilo

Einstein was right: "Two things are unlimited: the universe and the human stupidity. But i'm not quite sure about the former..."

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

DO1THL wrote:
But what about eating hats then?

Image result for edible hat

Edible Party Hat http://www.instructables.com/id/...

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I always use {} on if-else statements because I find it easier to follow the branching flow.
 

if ( )
    {
    }
else
    {
    if ( )
        {
        }
    else
        {
        };
    };
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

mikech wrote:
I always use {}...
yes

David (aka frog_jr)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:

UART logging/debugging is harder to implement, but can be richer on information. Of-course, if your UART(s) is(are) already occupied then this is not the way.

 

Going to concur with this one.  (almost) all my AVR gizmos have a UART, and it has been so profitable to stick in code that says:

 

When you execute '1', send '1' to the serial port

When you execute '2', send '2' to the serial port

When you execute '3', send '3' to the serial port

When you execute '4', send '4' to the serial port

When you execute '5', send '5' to the serial port

When you execute '6', send '6' to the serial port

 

(I tried to put that into the <code> window, but as usual it failed.)

 

Expecting 123456 and you get 12333333333... is a pretty good indication of where the problem is.

 

S.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Pretend that we're all really stupid and just beginning to know how a microcontroller works.  Pretend that we all have no idea what "icmp_header->type[0] == ICMP_REP" could possibly mean.  In C language, anything with a "->" after it is is a pointer [a 16-bit integer that refers to an address that is located in AVR SRAM] to a structure.  This structure could possibly have an array in it.  This array may be called "type" or it may a  type called "type" or it may be a definition of a type of type[] type with value type of type that dereferences type with value pointing to type...

   Why do "programmers" do this?  Is this something that they learned in school?  Did someone, somewhere tell them that this was some kind of elegance? 

   Programming lesson ONE:   Never label a variable "type".

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
if your UART(s) is(are) already occupied then this is not the way.

If you don't have a UART spare for logging, then you don't have enough UARTs!

 

http://www.avrfreaks.net/comment...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Simonetta wrote:

Pretend that we're all really stupid

 

To be fair, some of us, myself included, are really stupid.  And I'm pretty sure that even the most brilliant among us can identify a moment in which we were really stupid.

 

Doesn't take pretending.

 

S.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

<-I’m with stupid

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Fianawarrior wrote:
Thanks guys for the advise, I learned a lot last night

So.. Is the problem solved? If so, what was it?

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
Is the problem solved? If so, what was it?

And, if it is, mark it: http://www.avrfreaks.net/comment...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

mikech wrote:
I always use...

 

Do you always use superfluous null statements, too?  [someday it will burn you...]

mikech wrote:
};

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I do.
The C language does not require a ; terminator for a {} block, but I find that it is a nice visual indicator of where the statement actually ends.
My caffeine level is low at the moment and I am having difficulty in envisioning scenario(s) where superfluous null statements will 'burn' me.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If you don't have a UART spare for logging...

Of course if you don't have a spare USART, but you have a little bit of free memory, then all it takes is one pin for a software UART.

 

One of my first AVR projects was an LCD backpack, and it has occasionally served well as a one pin "debugger", especially on projects that don't otherwise have an LCD.

 

JC 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I eat my hat last night, too much to drink.  still feel ill and it 12:40am the next day.

 

The working version is:

void ip_protocol_handler(struct ETHERNET_HEADER * ethernet_header){
 
 struct IP_HEADER * ip_header = (struct IP_HEADER *)&ethernet_header->frame;
 struct ICMP_HEADER * icmp_header = (struct ICMP_HEADER *)&ip_header->data;
 
 Bool flags = false;
 
 if(ip_header->frags[0] & 0x20)
  flags = true;
 
 int offset = (ip_header->frags[0] & 0x1f) << 8; offset |= ip_header->frags[1];
 
 if((flags == false) && (offset == 0)){
  
  if(icmp_header->type[0] == ICMP_REQ)
   respond_to_ping(ethernet_header);
   
  else if((icmp_header->type[0] == ICMP_REP) || (icmp_header->type[0] == ICMP_UNREACH))
   _handle_ping_response(ip_header);
 }
 
 else if((flags == true) && (offset == 0)){
  
  // fist packet
  insert_packet_fragment(flags, 0, ethernet_header); 
 }
 
 else if(offset != 0){
  
  // packets
  insert_packet_fragment(flags, (offset / 0xb9), ethernet_header);
 }
}

The not working version was

 

void ip_protocol_handler(struct ETHERNET_HEADER * ethernet_header){
 
 struct IP_HEADER * ip_header = (struct IP_HEADER *)&ethernet_header->frame;
 struct ICMP_HEADER * icmp_header = (struct ICMP_HEADER *)&ip_header->data;
 
 Bool flags = false;
 
 if(ip_header->frags[0] & 0x20)
  flags = true;
 
 int offset = (ip_header->frags[0] & 0x1f) << 8; offset |= ip_header->frags[1];
 
 if((flags == false) && (offset == 0)){
  
  if(icmp_header->type[0] == ICMP_REQ)
   respond_to_ping(ethernet_header);
   
  else if((icmp_header->type[0] == ICMP_REP) || (icmp_header->type[0] == ICMP_UNREACH))
   _handle_ping_response(ip_header);
 }
 
 else if((flags == true) && (offset == 0)){
  
  // fist packet
  insert_packet_fragment(flags, 0, ethernet_header); 
 }
 
 else if(flags && offset != 0){
  
  // packets
  insert_packet_fragment(flags, (offset / 0xb9), ethernet_header);
 }
 else if(!flags && offset != 0){
  
  // packets
  insert_packet_fragment(!flags, (offset / 0xb9), ethernet_header);
 }
}

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for highlighting the difference. I'm a bit worried about your 'fist packet'. Might make you a bit sore after a night of drinking, especially if the sleeper hold is applied. Refer to Chappelle's "mad real world".

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

"Highlighting the difference" !
I had to copy and run a text-comparer to discover that the logic had changed at line 28 (in post #31) and that the original 'problem' from post #1 was at line 18.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

DocJC wrote:

If you don't have a UART spare for logging...

Of course if you don't have a spare USART, but you have a little bit of free memory, then all it takes is one pin for a software UART

Or, if you have some free memory, write debug info to it - then extract that using the debugger.

 

Often useful to use said memory in the form of a ring buffer (aka circular buffer or FIFO).