Verbetering in C-code

Op 23 oktober 2020 19:29:17 schreef tysie:
Dus het gaat er niet om, om de functie checkSensors() korter te maken, maar vooral de functie checkSensor().

Let op de naamgeving!
Geef functies duidelijk herkenbare en onderscheidbare namen. Ik moest 80x lezen voordat ik door had dat het twee verschillende functies zijn.
Nu snap je het nog wel, maar je gaat geheid de mist in.

Een (static) array gebruiken, dan hou je nog 1 regel code over per switch.

Geen idee wat dit is: LATAbits.LATA2 ?

Maar als dat constantes zijn, ergens boven in:

static const int gnds[] = { GNDS1, GNDS2, GNDS3, GNS4 }

De eerste switch wordt dan:

gnds[s-1] = 1;

De tweede kun je op een soortgelijke manier oplossen.

Note: Die eerste functie ziet er zo raar uit, dat er waarschijnlijk ook bugs in zitten.

En niet beginnen op 1 te tellen maar op 0, scheelt weer -1 in de andere functie die er dan uit kan.

Eens met ohm pi.

Ik vind de switch aanpak meestal erg lelijk.

Je kan hier perfect een array gebruiken. In pseudo code want mijn C is niet sterk.

.....


array gnds = .... zoals hieronder  ..
array sens = [ NULL,
               PORTAbits.RA3,   
               PORTAbits.RA1,
               PORTBbits.RB5,     
               PORTBbits.RB3 ]


 Checksensor(s) {
  
    gnds[s] = 1;
    delay;
    return sens[s];

 }

Soms kan het zo simpel zijn.
Ik wist niet dat je dat ook in een array kon plaatsen.

Hoe weet ik wat voor datatype de array moet hebben?

Zo gemakkelijk gaat dat niet volgens mij.

Een array is op zich wel leuk, maar PORTAbits is volgens mij een SFR en RA3 een bit binnen dat register. Die kun je niet zomaar in een array zetten.

Ik ken de PIC niet zo, maar het ziet er uit alsof hier bitfield struct 's zijn gebruikt om individuele bits te adresseren. Best wel clever, maar die ga je nooit zomaar in een array kunnen zetten.

Een array element zal dan een pointer naar de SFR en een bitmask voor het bit moeten krijgen. Hoe dat er in code uitzit hangt af van hoe de definities van die SFR's er uit zien.

[Bericht gewijzigd door deKees op vrijdag 23 oktober 2020 22:01:18 (19%)

Je hebt gelijk

https://www.microchip.com/forums/m/tm.aspx?m=1027427&p=1

Moet toch wat anders bedenken

Als je alle sense ingangen op poortA zet en alle outputs op PoortB kan het een stuk korter.

Hum, zijn dus methods in een class? Ik had gehoopt op class constantes.
Wordt wat lastiger dus.
(Kunnen ook methods in een struct zin trouwens).
Waarschijnlijk is de = overloaded in die class.

Heeft iemand de header files bij de hand waar die dingen gedefinieerd zijn?

[Bericht gewijzigd door henri62 op zaterdag 24 oktober 2020 00:52:41 (48%)

Op 24 oktober 2020 00:45:49 schreef henri62:
Heeft iemand de header files bij de hand waar die dingen gedefinieerd zijn?

De definitie stond in link van tysie boven je post. Het is geen struct, het is is geen class, het is een bitfield
Zie https://en.wikipedia.org/wiki/Bit_field

De I/O in een PIC is memory mapped, dus PORTb.RBO geeft aan bit 0 op addres 0x05. Een regel als PORTB.RB0 = 0 wordt vertaald door de compiler met de instructie BCF 0x05,0 (Clear bit 0 op het adres 0x05).

Nee, het zijn geen classes en geen methods en er zijn ook geen overloaded operators. Dit is standaard C (werkt ook in C++) en zijn bitfield structs.

Dat gaat zo:

typedef struct
{  unsigned int Bit1 : 1;
   unsigned int Bit2 : 1;
   unsigned int Bit3 : 1;
   unsigned int Bit4 : 1;
   unsigned int Bit5 : 1;
   unsigned int Bit6 : 1;
   unsigned int Bit7 : 1;
   unsigned int Bit8 : 1;
} Bitfield;

De ': 1' geeft aan dat het een unsigned int is van 1 bit en in dit geval worden ze alle 8 samen in een enkele byte opgeslagen. De compiler doet dan automatisch de benodigde bit masking en bitshifts.

De applikatie kan de bits gebruiken alsof het gewone struct variabelen zijn. Al kan de waarde in dit geval allen 1 of 0 zijn

   Bitfield B1;
   B1.Bit1 = 1;
   B1.Bit2 = 0;
   B1.Bit3 = B1.Bit1;

Probleem is wel dat de compiler vrij is om de volgorde van de bits te kiezen en om gaps tussen te voegen. Dat maakt deze methode minder portable en daardoor ook minder populair. In de praktijk ligt de volgorde wel vast zolang ze altijd dezelfde compiler gebruikt.

Op 24 oktober 2020 09:09:20 schreef JoWi:
[...]
De I/O in een PIC is memory mapped, dus PORTb.RBO geeft aan bit 0 op addres 0x05. ...

Ah, snap ik.

Dan kun je dus een arraytje maken met 4 structs erin (address, bitnummer).
Wel een beetje lelijk omdat je de RB0 definitie niet kunt gebruiken en dus geen gebruik kunt maken van de header files. (misschien met offset off ??)

struct pin {
  uint8_t address;
  uint8_t bit;
};

const struct pin gnds[] = {
    LATAbits, 2,  // GNDS1
    LATAbits, 0,  // GNDS2
    LATBbits, 4,  // GNDS3
    LATBbits, 2,  // GNDS4
};
ipv de 1-ste switch wordt dit zoiets:

gnds[s-1].address |= (1 << gnds[s-1].bit);

En de 2-de switch:

gnds[s-1].address &= ~(1 << gnds[s-1].bit);

Maar voor een leek wordt het er niet simpeler op.

Op zich niks mis met zo een switch. Alleen zou ik die wel wat compacter noteren.

Iets als :

unsigned char checkSensor(unsigned char s)
{
    unsigned char r = false;
    
    switch(s)
    {
        case 1: GNDS1 = 1; break;
        case 2: GNDS2 = 1; break;
        case 3: GNDS3 = 1; break;
        case 4: GNDS4 = 1; break;
    }
    __delay_ms(1);
    switch(s)
    {
        case 1: r = SENS1; break;
        case 2: r = SENS2; break;
        case 3: r = SENS3; break;
        case 4: r = SENS4; break;
    }
    GNDS1 = GNDS2 = GNDS3 = GNDS4 = 0;
    return r;
}

Dat was ook mijn idee om nog te posten.

Maar als we toch bezig zijn: geen idee wat die andere functie nou precies zou moeten doen.

[Bericht gewijzigd door henri62 op zaterdag 24 oktober 2020 20:02:13 (55%)