Mega32: Program hängt.

[Frank G. (dg1sbg)]

Hallo !

Ich habe ein lange funktionierendes Programm für einen Mega32 erweitert. 
Nun läuft das Ding zwar an, aber es bleibt an einer bestimmten Stelle 
hängen. Das hatte ich früher schon mal - zu viel Speicher benötigt.

Derzeit sieht die Ausgabe des avr-gcc so aus:

Linking: psuctrl.elf
avr-gcc -mmcu=atmega32 -I. -gdwarf-2 -DF_CPU=8000000UL  -Os -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -Wall -Wstrict-prototypes -Wa,-adhlns=controller.o -I/Users/frgo/gnc//avr/gaul -std=gnu99 -MD -MP -MF .dep/psuctrl.elf.d controller.o --output psuctrl.elf -Wl,-Map=psuctrl.map,--cref   /Users/frgo/gnc//avr/gaul/libgaul.a -Wl,-u,vfprintf -lprintf_flt  -lm

Creating load file for Flash: psuctrl.hex
avr-objcopy -O ihex -R .eeprom psuctrl.elf psuctrl.hex

Creating load file for EEPROM: psuctrl.eep
avr-objcopy -j .eeprom --set-section-flags=.eeprom="alloc,load" \
  --change-section-lma .eeprom=0 -O ihex psuctrl.elf psuctrl.eep
avr-objcopy: --change-section-lma .eeprom=0x00000000 never used

Creating Extended Listing: psuctrl.lss
avr-objdump -h -S psuctrl.elf > psuctrl.lss

Creating Symbol Table: psuctrl.sym
avr-nm -n psuctrl.elf > psuctrl.sym

Size after:
psuctrl.elf  :
section             size      addr
.text              22422         0
.data                692   8388704
.bss                 863   8389396
.stab               1632         0
.stabstr             198         0
.debug_aranges       512         0
.debug_pubnames     4201         0
.debug_info        24239         0
.debug_abbrev       6788         0
.debug_line        19513         0
.debug_frame        3104         0
.debug_str          5322         0
.debug_loc         13478         0
.debug_ranges       1104         0
Total             104068

AVR Memory Usage:
-----------------
Device: atmega32

Program:   23114 bytes (70,5% Full)
(.text + .data + .bootloader)

Data:       1555 bytes (75,9% Full)
(.data + .bss + .noinit)

-------- end --------

Angehängte Dateien:
controller.c/.h: Der eigentliche Source Code.
libgaul.tar: TAR Datei mit Utility Library (wird auch in anderen 
Projekten erfolgreich eingesetzt)
Makefile: ...

Sieht also doch gut aus, oder?

Bin für jeden Hinweis dankbar !!!

Viele Grüße

  Frank DG1SBG

[Marcus K. (marcusk)]

hier sind ein paar stellen die mir merkwürdig vorkommen:

> tRelayID get_relay_id( tRelayID nID )
> {
>   return (tRelayID) nID;
> }

der sinn des cast bzw der gesamten funktion ist mir nicht ganz klar

> uint8_t  nTmpSREG = SREG;      // Save status
dafür ist der compiler da, das macht man doch nicht selber

damit könntest du schon etwas sparen.

[gast (Gast)]

> tRelayID get_relay_id( tRelayID nID )
> {
>   return (tRelayID) nID;
> }


das ist echt witzig ^^

[Frank G. (dg1sbg)]

Marcus Kunze schrieb:
> hier sind ein paar stellen die mir merkwürdig vorkommen:
>
>> tRelayID get_relay_id( tRelayID nID )
>> {
>>   return (tRelayID) nID;
>> }
>
> der sinn des cast bzw der gesamten funktion ist mir nicht ganz klar

Reine Abstraktion. Das könnte ein paar Byte sparen, wenn dies 
weggelassen würde. Aber: Meine Frage ist ein wenig anders gelagert.

Lt. Compiler habe ich noch Platz ...

>> uint8_t  nTmpSREG = SREG;      // Save status
> dafür ist der compiler da, das macht man doch nicht selber

Aha. Und wie?

>
> damit könntest du schon etwas sparen.

Muss ich denn???

Danke!

Frank

[Frank G. (dg1sbg)]

gast schrieb:
>> tRelayID get_relay_id( tRelayID nID )
>> {
>>   return (tRelayID) nID;
>> }
>
>
> das ist echt witzig ^^

Nicht unbedingt. Das ist die Umsetzung zwischen Software-Ebene und 
Hardware-Ebene. Wenn  ich die Relays anders beschalte bzw. verdrahte, 
kann ich hier die Addressierung zentral ändern. Zur Zeit bedeutet die 
Funktion: Die Relays sind so mit Nummern versehen, dass diese sich 
direkt als das Bit des Ports nehmen lassen. Das muss nicht immer so 
sein, v.a. wenn das selbe Grund-Programm für mehrere ähnliche Projekte 
verwendet wird.

Das ist einfach eine Abstraktionsebene (mehr).

Gruß,
  Frank

P.S. Ich lach auch gerne - nur gerade eben nicht mehr. Das Programm 
läuft immer noch nicht. ;-)

[Marcus K. (marcusk)]

>> uint8_t  nTmpSREG = SREG;      // Save status
> dafür ist der compiler da, das macht man doch nicht selber

Aha. Und wie?

einfach weglassen, damit sparst du schon mal 1byte ram

[Stefan B. (stefan)]

Du hast das SRAM zu fast 76% mit Variablen gefüllt und zusätzlich in 
einigen Funktionen ziemlich viel lokal auf dem Stack liegen.

Ich würde da abspecken oder versuchsweise auf den pinkompatiblen, aber 
grösseren Atmega644P wechseln 
(http://www.atmel.com/dyn/resources/prod_documents/doc8001.pdf)

[Frank G. (dg1sbg)]

Marcus Kunze schrieb:
>>> uint8_t  nTmpSREG = SREG;      // Save status
>> dafür ist der compiler da, das macht man doch nicht selber
>
> Aha. Und wie?
>
> einfach weglassen, damit sparst du schon mal 1byte ram

???
Äh - wie kann denn der Compiler ahnen, dass ich das SREG sichern 
will/muss ?

Es gibt da wohl inzwischen ein Makro, ja, aber weglassen ? (Ich bin nur 
Gelegenheits-µC-Programmierer - ich komm einfach nicht drauf, wie der 
Compiler das ohne irgendwas hinbekommt).

Danke!

Gruß,
  Frank

[Peter (Gast)]

Was ist besser zu lesen?

rc_t switch_it( tComponentClassID nObjectClassID, char **ppcTokens, uint16_t nTokenIndex )
{
  rc_t              nRC            = RC_OK;
  tComponentClassID nClassID       = OBJECT_CLASS_ID_INVALID_ID;
  uint16_t          nCurTokenIndex = nTokenIndex;
  char             *pcCurToken     = ppcTokens[ ++nCurTokenIndex ];

  nRC = get_key_value_pair( pcCurToken, acKey, acValue );

  if( nRC == RC_OK )
  {
    // Value -> Component Class
      
    nClassID = get_component_class_id( acKey );
    if( nClassID == nObjectClassID )
    {
      tIOID nIOID 
        = get_io_id_from_darc_value( nObjectClassID, acValue );
          
      if( nIOID != NO_IO )
      {
        // Next token = Key/value pair for mode
            
        pcCurToken = ppcTokens[ ++nCurTokenIndex ];

        nRC = get_key_value_pair( pcCurToken, &acKey, &acValue );
        if( nRC == RC_OK )
        {
          tIOStatus nMode = get_mode( acValue );
          switch_io( nObjectClassID, nIOID, nMode );
        }
        else
        {
          nRC = DARC_CAT_CMD_FC_INVALID_PARAMETER;
        }
      }
      else
      {
        nRC = DARC_CAT_CMD_FC_INVALID_PARAMETER;
      }
    }
    else
    {
      nRC = DARC_CAT_CMD_FC_INVALID_PARAMETER;
    }
  }
  else
  {
    nRC = DARC_CAT_CMD_FC_INVALID_PARAMETER;
  }
 
  return nRC;
}

oder

rc_t switch_it( tComponentClassID nObjectClassID, char **ppcTokens, uint16_t nTokenIndex )
{
  rc_t              nRC            = RC_OK;
  tComponentClassID nClassID       = OBJECT_CLASS_ID_INVALID_ID;
  uint16_t          nCurTokenIndex = nTokenIndex;
  char             *pcCurToken     = ppcTokens[ ++nCurTokenIndex ];

  nRC = get_key_value_pair( pcCurToken, acKey, acValue );
  if( nRC != RC_OK ) {
    return DARC_CAT_CMD_FC_INVALID_PARAMETER
  }

  // Value -> Component Class
  nClassID = get_component_class_id( acKey );
  if( nClassID != nObjectClassID ) {
     return DARC_CAT_CMD_FC_INVALID_PARAMETER
  }

  tIOID nIOID = get_io_id_from_darc_value( nObjectClassID, acValue );
  if( nIOID == NO_IO ) {
     return DARC_CAT_CMD_FC_INVALID_PARAMETER;
  }

  // Next token = Key/value pair for mode
  pcCurToken = ppcTokens[ ++nCurTokenIndex ];

  nRC = get_key_value_pair( pcCurToken, &acKey, &acValue );
  if( nRC == RC_OK ) {
     return DARC_CAT_CMD_FC_INVALID_PARAMETER;
  }

  tIOStatus nMode = get_mode( acValue );
  switch_io( nObjectClassID, nIOID, nMode );

  return RC_OK ;
}

Ja ich weiss, es gibt Leute die sagen es darf nur ein return geben, aber 
ich das ist mal ein schönes Beispiel dafür das es das ganze nicht 
lesbarer macht.

[Stefan E. (sternst)]

Frank Goenninger schrieb:
> Marcus Kunze schrieb:
>>>> uint8_t  nTmpSREG = SREG;      // Save status
>>> dafür ist der compiler da, das macht man doch nicht selber
>>
>> Aha. Und wie?
>>
>> einfach weglassen, damit sparst du schon mal 1byte ram
>
> ???
> Äh - wie kann denn der Compiler ahnen, dass ich das SREG sichern
> will/muss ?

Vergiss das einfach. Marcus hat nur nicht durchschaut, was du da machst. 
Ich würde an deiner Stelle allerdings <util/atomic.h> benutzen.

[Frank G. (dg1sbg)]

Stefan B. schrieb:
> Du hast das SRAM zu fast 76% mit Variablen gefüllt und zusätzlich in
> einigen Funktionen ziemlich viel lokal auf dem Stack liegen.

Yep. Das ist sicherlich das Problem ...

>
> Ich würde da abspecken oder versuchsweise auf den pinkompatiblen, aber
> grösseren Atmega644P wechseln
> (http://www.atmel.com/dyn/resources/prod_documents/doc8001.pdf)

Ist bestellt ;-)

Danke!

Gruß,
  Frank

[Frank G. (dg1sbg)]

Peter schrieb:

> rc_t switch_it( tComponentClassID nObjectClassID, char **ppcTokens,
> uint16_t nTokenIndex )
> {
>   ....
>   // Next token = Key/value pair for mode
>   pcCurToken = ppcTokens[ ++nCurTokenIndex ];
>
>   nRC = get_key_value_pair( pcCurToken, &acKey, &acValue );
>   if( nRC == RC_OK ) {
>      return DARC_CAT_CMD_FC_INVALID_PARAMETER;
>   }
>
>   tIOStatus nMode = get_mode( acValue );
>   switch_io( nObjectClassID, nIOID, nMode );
>
>   return RC_OK ;
> }

Stimmt. Übernommen. Nur: Programm damit auch nicht kleiner geworden ;-)

Gruß,
  Frank

[Frank G. (dg1sbg)]

Stefan Ernst schrieb:
> Frank Goenninger schrieb:
>> Marcus Kunze schrieb:
>>>>> uint8_t  nTmpSREG = SREG;      // Save status
>>>> dafür ist der compiler da, das macht man doch nicht selber
>>>
>>> Aha. Und wie?
>>>
>>> einfach weglassen, damit sparst du schon mal 1byte ram
>>
>> ???
>> Äh - wie kann denn der Compiler ahnen, dass ich das SREG sichern
>> will/muss ?
>
> Vergiss das einfach. Marcus hat nur nicht durchschaut, was du da machst.
> Ich würde an deiner Stelle allerdings <util/atomic.h> benutzen.

Jau. Sicherlich. Der Ur-Quelltext stammt noch aus einer Zeit, als es 
atomic.h noch nicht gab...

Danke übrigens - ich dachte schon, ich hätte was Grundlegendes nicht 
mitbekommen.

Gruß,
  Frank

[Michael G. (linuxgeek)]

Hast Du auch mal an die Stack-Variablen gedacht? 70% full heisst hier 
statische Variablen, da bleibt nicht allzu viel fuer den Stack.