uSupply Firmware Released

[EEVblog]

I've now released the uSupply firmware for those interested.

https://gitlab.com/eevblog/usupply-firmware
https://gitlab.com/eevblog/usupply-library
https://gitlab.com/eevblog/usupply-usb

[uer166]

Is it just my lack of experience/exposure to it or is C++ harder to read/understand than C in the embedded context?

[EEVblog]

Is it just my lack of experience/exposure to it or is C++ harder to read/understand than C in the embedded context?

To me it looks like Klingon. So yeah, harder. But mostly depends on how you write it.

[NivagSwerdna]

Code: [Select]

const char * volatile current{
[]() -> const char *
{
I'm pretty sure it is Klingon. Live long and prosper.

[Dave]

Very professionally organized repositories.
Can't say I'm a big fan of C++ for embedded, but it does have its place in safety non-critical applications.

[SiliconWizard]

Code: [Select]

const char * volatile current{
[]() -> const char *
{
I'm pretty sure it is Klingon. Live long and prosper.

 

Yeah. Some C++ constructs are really cryptic.

I've taken a look at the source code. Overall, it looks pretty well written. I won't comment on the use of C++ (we have done this a lot already), but I admit that, although the whole thing looks very well written, it's still a bit hard to grasp for anyone not having developed it IMHO.

Another thing that strikes me is the amount of source code for all this. I don't have all the features of the uSupply in mind,  but it sure looks like there is *a lot* of code, and I'd venture that it could have been done with a lot less code. In that respect, having used C++ doesn't seem like a gain in development time IMHO.

Still - hats off to Dave (2). I have rarely seen code that was that well written and laid out (not judging functionality or possible bugs of course, it's just a first look.)

[nuclearcat]

I think when device is released and if it will be affordable, we might see competition who can (IF CAN!) write better code (and hopefully giving away to Dave)

[NivagSwerdna]

I think when device is released and if it will be affordable, we might see competition who can (IF CAN!) write better code (and hopefully giving away to Dave)

Probably but I've never really understood working for free, seems like a poor business model. 

[ataradov]

You do not work for free, you work for yourself. If you think a better firmware will be useful to you, you work on it. Releasing it to others is just a side effect. It is mostly driven by the fact that it is very hard to monetize things like this.

And yes, this C++ heavy code is very hard to read, and the whole functionality could be implemented in a much-much smaller code.

[thm_w]

These are the compiler flags, I believe:

set(CMAKE_CXX_FLAGS           "${COMMON_FLAGS} -std=c++1z -fno-rtti -fno-exceptions -fno-use-cxa-atexit -fno-threadsafe-statics -ftemplate-backtrace-limit=0" CACHE INTERNAL "cpp compiler flags")

So it should be unnecessary to tag a bunch of functions with noexcept right?

Code: [Select]

void Enabled( bool enable_1, bool enable_2 ) noexcept

Is it just my lack of experience/exposure to it or is C++ harder to read/understand than C in the embedded context?

Its not necessarily all C++, its how this code was written using a number of advanced C++ features.

[EEVblog]

And yes, this C++ heavy code is very hard to read, and the whole functionality could be implemented in a much-much smaller code.

Not a comparison because the features aren't even remotely the same, but for those interested, previous versions of my uSupply written by me are 389 lines and 479 lines for two of the design variants.
That's everything, including comments, UI code, control code etc.
Not sure I'd like to guess how many lines this version of the uSupply would have taken me in "Dave C", but yes, it would been vastly different to the way David writes his code. I just let him do it the way he wanted, we have vastly different styles. But he writes games engines in his spare time for fun, and I wouldn't know C++ if it bit me on the arse.

[EEVblog]

For kicks, this is "Dave C" for my last uSupply USB prototype, the entire program:

Code: [Select]

/* ****************************************
uSuppyUSB Firmware
Version 1.0
Last Edit: 8/9/12
Written in: AVR GCC C under AVR Studio 6
Written by: David L. Jones
Copyright(C)2012 David L. Jones
[url=http://www.eevblog.com]www.eevblog.com[/url]
NOTES:
- Written for an ATmega88A target on Rev 1 uSupplyUSB Hardware

Revision notes
1.0 - First prototype version

******************************************* */

#define F_CPU 8000000 //clock frequency 8MHz

#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>
#include <avr/eeprom.h>
#include <string.h>
#include <stdio.h>

//definitions
#define version 1
#define TRUE    1
#define FALSE   0
#define CURRbutton (PINB & 1<<PB7) //CURRENT button
#define SETbutton (PINB & 1<<PB6) //SET button

#define bit_get(p,m) ((p) & (m))
#define bit_set(p,m) ((p) |= (m))
#define bit_clear(p,m) ((p) &= ~(m))

//#define POWPIN_HIGH PORTC.PINC4=1
//#define POWPIN_HIGH bit_set(PORTC, PINC4);
#define POWPIN_HIGH PORTC|=0b00010000
#define POWPIN_LOW bit_clear(PORTC, PINC4);


#define DIG1ON          PORTB=0xE //Switch ON the first digit, current display
#define DIG2ON          PORTB=0xD //Switch ON the second digit, current display
#define DIG3ON          PORTB=0xB //Switch ON the third digit, current display
#define DIGITSOFF       PORTB=0xF    //Switch OFF all digits, current display

#define DIG1VON         PORTB=0xFB //Switch ON the first digit, voltage display
#define DIG2VON         PORTB=0xF7 //Switch ON the second digit, voltage display
#define DIG3VON         PORTB=0xEF //Switch ON the third digit, voltage display
#define DIGITSVOFF      PORTB=0xFF    //Switch OFF all digits, current display

#define SEGPORT PORTD // the LED display digits are on PORTD

// define the two rotary encoder input pins
#define PHASE_A (PINC & 1<<PC5)
#define PHASE_B (PINC & 1<<PC6)


#define ADCcomplete (ADCSRA & (1<<ADSC)) //TRUE if ADC is complete
//#define ADCcomplete (ADCSRA & 1<<ADIE) //TRUE if ADC is complete
#define setADCIE (ADCSRA|=1<<ADIE)

//global variables
unsigned char DIGIT_NUM;
int  SET_VOLTAGE; //the set output voltage in mV
int  SET_CURRENT; //the set output current limit mA
int  READ_VOLTAGE; //the measured output voltage in mV
int  READ_CURRENT; //the measured current output in mA
int  READ_VOLTAGE_ARRAY[40]; //array for averages
int  READ_CURRENT_ARRAY[40]; //array for averages
int  READ_VOLTAGE_AVERAGE; //the measured output voltage in mV
int  READ_CURRENT_AVERAGE; //the measured current output in mA
unsigned char digit; //the number of the currently displayed LED digit
unsigned char CCmode; //TRUE if in Constant Curent mode
int  VoltageSETcount; //counter for temp display of the set voltage as the knob is turned.
int  SETbuttonCount; //counter for the time the SET button has been pressed
int  DPblinkCount; //counter for the constant current mode decimal point blink mode
char blink;

const unsigned char digitdata[11]={0x3F,0x6,0x5B,0x4F,0x66,0x6D,0x7D,0x7,0x7F,0x6F,0}; // 7 segment digit display data

volatile int8_t enc_delta; // -128 ... 127
static int8_t last;
static int8_t knobcount;


//*****************************************************************
//saves the current voltage and current figures to EEPROM
void SaveEEPROM(void)
{
cli(); //disable interrupts so it doesn't corrupt the write process
// eeprom_write_word(0,SET_VOLTAGE);
// eeprom_write_word(2,SET_CURRENT);
sei(); //enable interrupts again
return;
}

//*****************************************************************
//loads the stored voltage and current data from the EEPROM
void LoadEEPROM(void)
{
cli();
// SET_VOLTAGE=eeprom_read_word(0);
// SET_CURRENT=eeprom_read_word(2);
sei();
return;
}

//*****************************************************************
//displays a number on the selected 7seg display
// If DP is TRUE the associated Decimal Point turns ON
void Disp7SEG(unsigned char digit, unsigned char num, unsigned char DP)
{
unsigned char temp;

DIGITSOFF; //disable the entire LED display
DIGITSVOFF;

// if ((CCmode!=0) && (digit<4)) DP=1; //force decimal point on for current display in CCmode

if (num>9) num=10; // if unknown digit then display a blank digit
temp=digitdata[num];
if (DP!=0) temp=temp|128; // turn on decimal point if requested
SEGPORT=temp; //output the display segments

switch(digit)
{
case 1: DIG1ON; break;
case 2: DIG2ON; break;
case 3: DIG3ON; break;
case 4: DIG1VON; break;
case 5: DIG2VON; break;
case 6: DIG3VON; break;
}
//leave the digit displayed until the next call, so we can continue processing and we get persistence of vision.
return;
}


//*****************************************************************
// Updates the voltage and current PWM outputs
void UpdateOutput(void)
{
unsigned long PWMvalue;

PWMvalue=((unsigned long)SET_VOLTAGE*1000)/47695; //(12210mV full scale / 256 bits)
if (PWMvalue>255) PWMvalue=255; //overflow trap, we don't want any undesirable wrap
OCR1A=PWMvalue; //set the output VOLTAGE

PWMvalue=((unsigned long)SET_CURRENT*1000)/1078; //0-255 = 0-275mV/
if (PWMvalue>255) PWMvalue=255; //overflow trap, we don't want any undesirable wrap
OCR1B=PWMvalue; //set the output CURRENT

return;
}


//*****************************************************************
// reads the voltage and current values from the ADC and updates the global variables
void ReadValues(void)
{
unsigned long value;
unsigned long total;
unsigned char c,temp;

if (CURRbutton)
{
//measure VOLTAGE
ADMUX=0b00000001; //select ADC1 input and 1.1V reference, right adjusted

value=ADCL; //get the upper 8 bit result
value=value+(ADCH*256); //get the lower 8 bit result and add it on.
value=value*1074; //convert to microvolts for 1.1V reference
value=value/1000; //convert back to millivolts
READ_VOLTAGE=value*12; //multiply for the resistor divider

for(c=0;c<39;c++) //update the moving average array
READ_VOLTAGE_ARRAY[c]=READ_VOLTAGE_ARRAY[c+1];
READ_VOLTAGE_ARRAY[39]=READ_VOLTAGE;

total=0;
for(c=0;c<40;c++)
total=total+READ_VOLTAGE_ARRAY[c];
READ_VOLTAGE_AVERAGE=total/40;
}
else
{
//measure CURRENT
total=0;
ADMUX=0; //select ADC0 input and 1.1V reference, right adjusted

value=ADCL; //get the upper 2 bit result
value=value+(ADCH*256); //get the lower 8 bit result and add it on.
value=value*10740; //convert to microvolts for 1.1V reference
value=value/10000; //convert back to mV (/1000)
//the final value is now in mA*10
READ_CURRENT=value;

for(c=0;c<39;c++) //update the moving average value array
READ_CURRENT_ARRAY[c]=READ_CURRENT_ARRAY[c+1];
READ_CURRENT_ARRAY[39]=READ_CURRENT;

total=0;
for(c=0;c<40;c++)
total=total+READ_CURRENT_ARRAY[c];
READ_CURRENT_AVERAGE=total/40;
}

return;
}

//*****************************************************************
// Checks to see if the user has turned the rotary encoder knob
void CheckKnob(void)
{
if (enc_delta==1)
{
knobcount++;
VoltageSETcount=0;
}
if (enc_delta==-1)
{
knobcount--;
VoltageSETcount=0;
}
enc_delta=0;

if (knobcount>3)
{
if (CURRbutton) //adjust the VOLTAGE
{
SET_VOLTAGE=SET_VOLTAGE-50;
if (SET_VOLTAGE<0) SET_VOLTAGE=0;
}
else //adjust the CURRENT
{
SET_CURRENT=SET_CURRENT-1;
if (SET_CURRENT<0) SET_CURRENT=0;
}
UpdateOutput(); //update the output voltage/current on the terminals
knobcount=0;
}

if (knobcount<-3)
{
if (CURRbutton) //adjust the VOLTAGE
{
SET_VOLTAGE=SET_VOLTAGE+50;
if (SET_VOLTAGE>12200) SET_VOLTAGE=12200;
}
else //adjust the CURRENT
{
SET_CURRENT=SET_CURRENT+1;
if (SET_CURRENT>300) SET_CURRENT=300;
}
UpdateOutput(); //update the output voltage/current on the terminals
knobcount=0;
}

}

//*****************************************************************
// Interrupt Service Routine
// This ISR has several functions. It is based on TIMER0
// 1) Updates the 7 segment display at regular intervals
// (This display only one digit every interrupt, digits get cycled through each ISR pass)
// 2) Reads and updates the status of the rotary encoder knob
// 3) Ticks over a counter to hold the display in a set state for a certain period

ISR(TIMER0_OVF_vect)
{
unsigned int temp;
unsigned char dig1, dig2, dig3, dig4, dig5, dig6, dp1, dp2, dp3;
int8_t state, diff, CURRset;

cli(); //disable interrupts

VoltageSETcount++;
if (VoltageSETcount>=500) VoltageSETcount=500;

if (!SETbutton) SETbuttonCount++; else SETbuttonCount=0;

if (CCmode)
{
if (blink) DPblinkCount++; else DPblinkCount--;
if (DPblinkCount>100) blink=FALSE;
if (DPblinkCount<=0) blink=TRUE;
}

dp1=0; //turn all decimal points OFF
dp2=0;
dp3=0;
CURRset=0; //assume we are displaying the READ current and not the SET current

if (digit==4||digit==5||digit==6) //display the VOLATGE display digits
{
if (SETbutton) temp=READ_VOLTAGE_AVERAGE; else temp=SET_VOLTAGE;

if (VoltageSETcount<500) //force display of the SET voltage or current if the knob has been turned within the time limit
{
if (CURRbutton) temp=SET_VOLTAGE;
else
{
temp=SET_CURRENT;
CURRset=TRUE;
}
}


if (temp>9999) //check for voltages 10V or greater, we have to switch the decimal point and calcs
{
dig4=temp/10000;
temp=temp-(dig4*10000);
dig5=temp/1000;
temp=temp-(dig5*1000);
dig6=temp/100;
dp2=TRUE; //decimal point for 99.9
}
else
{
dig4=temp/1000;
temp=temp-(dig4*1000);
dig5=temp/100;
temp=temp-(dig5*100);
dig6=temp/10;
dp1=TRUE;
} //decimal point for 9.99

}
else // ELSE display the current display digits
{

if (SETbutton) temp=READ_CURRENT_AVERAGE; else temp=SET_CURRENT;

if (!CURRset && SETbutton) dp2=TRUE; //turn on decimal point for 99.9mA display if we are displaying the READ current
//otherwise display the SET current with 1mA resolution as 999mA

if (temp>=999) //if current is >99.9mA
{
dig1=temp/1000;
temp=temp-(dig1*1000);
dig2=temp/100;
temp=temp-(dig2*100);
dig3=temp/10;
}
else
{
dig1=temp/100;
temp=temp-(dig1*100);
dig2=temp/10;
temp=temp-(dig2*10);
dig3=temp;
}

if (CCmode) //if in constant current mode then blink the decimal point
{
if (blink)
{
dp1=FALSE;
dp2=FALSE;
dp3=FALSE;
}
}
}
//display the currently selected digit
//this digit will stay on until the next ISR loop, giving persistence of vision
if (digit==1) Disp7SEG(1,dig1,dp1);
if (digit==2) Disp7SEG(2,dig2,dp2);
if (digit==3) Disp7SEG(3,dig3,dp3);
if (digit==4) Disp7SEG(4,dig4,dp1);
if (digit==5) Disp7SEG(5,dig5,dp2);
if (digit==6) Disp7SEG(6,dig6,dp3);

digit++; //display the next digit next time through the ISR loop
if (digit>6) digit=1;

//read the rotary encoder knob
state = 0;
if( PHASE_A ) state = 3;
if( PHASE_B ) state ^= 1; // convert gray to binary
diff = last - state; // difference value
if( diff & 1){ // bit 0 = value (1)
last = state; // store new as next last
enc_delta += (diff & 2) - 1; // result will be +1 for positive direction and -1 for negative direction
}

ReadValues(); //read the ADC values

sei(); //enable interrupts again
}

//*****************************************************************
int main(void){
unsigned char c,d,average;
unsigned char keyspeed, keycount;

DDRB =0b00000111; //PB0, PB1, PB2, PB3, PB4 are OUTPUTS, all others INPUTS
PORTB=0b11111000; //enable pull-ups on all PORTB pins except PB1 and PB2 which are outputs
DDRC =0b00111000; //PORTC are all INPUTS, except for 3 LED digit outputs
PORTC=0b11000111; //enable pull-ups on only upper 3 pins PORTC pins
DDRD =0b11111110; //PORTD are all OUTPUTS
PORTD=0;

MCUCR=0; //Enable pin pull-ups and Disable Sleep BOD stuff.

POWPIN_HIGH; //Latch the power ON
//PORTC=0xFF;

// this must be done BEFORE the PWM is turned on to ensure we DON'T OVERSHOOT at power on!
LoadEEPROM();                   //restore the saved voltage and current settings
if (SET_VOLTAGE>12200) SET_VOLTAGE=12200; //do some out of bounds checking
if (SET_VOLTAGE<50) SET_VOLTAGE=50;
if (SET_CURRENT>100) SET_CURRENT=100;
if (SET_CURRENT<1) SET_CURRENT=1;

//set up TIMER0 used for the display updating (165Hz rate per digit)
TCCR0A=0x3; //use the /64 prescaler for clock source
TIFR0=0;
TCNT0=0x0;
TIMSK0=1; //enable the TIMER0 interrupt

//set up PWM in TIMER1
TCCR1A=0b10100001; //clear on compare match, SET at TOP for both PWM output pins. 8bit FAST PWM mode.
TCCR1B=0b00001001; //matchig 8bit fast PWM mode setting
TCNT1=0x3FE;

UpdateOutput(); //set the output voltage and current

//set up ADC
PRR=0; //make sure all periphers are turned ON
ADCSRB=0; //free running mode, manual trigger
DIDR0=3; //diable digital input on ADC0 and ADC1 pins
ADCSRA |= (1 << ADATE);
// ADMUX |= (1 << ADLAR); //left align ADC result
ADCSRA |= (1 << ADEN); //enable the ADC
ADCSRA |= (1 << ADSC); //start conversion
ADCSRA |= (1 << ADPS2); //prescaler = 128
ADCSRA |= (1 << ADPS1); //prescaler = 128
// ADCSRA |= (1 << ADPS0); //prescaler = 128

//set up the user control knob interrupt
// PCMSK1=32; //enable knob pin change interrupt
// PCICR=2; //ebale the PCIE1 pin change interrupt

Disp7SEG(3,version,0); //display version number at power on
_delay_ms(2000);

digit=1;
average=0;
keyspeed=0;
keycount=0;
VoltageSETcount=0;
SETbuttonCount=0;
DPblinkCount=0;

ReadValues();

  sei();        // enable interrupts (turns on display and functions)

while(TRUE)
{
if ((SET_CURRENT*10) < (READ_CURRENT_AVERAGE+1)) CCmode=TRUE; else CCmode=FALSE; //check to see if we are in Constant Current Mode
CheckKnob(); //check if the user has turned the knob
if (SETbuttonCount>=1500) //check to see if the SET button has been pressed for 3 seconds
{
SETbuttonCount=0;
SaveEEPROM();
cli();
Disp7SEG(1,5,0);
_delay_ms(1000);
sei();
}

}
}


[uer166]

Even in the tiny forum window I can figure out what's going on.. This must be hardware vs. software engineering thinking ways.

[Dave]

As soon as software becomes slightly more complex, you need to start splitting it in logical units, which you put into separate files. A well written software should not have files that stretch for more than a couple of hundred lines. There are of course always exceptions to this rule, but in general, one shouldn't have to scroll around and search for a particular part of code.

Next thing is abstraction. Just tossing register names around all over the place is very poor form. At the very least they should be macroed away with descriptive names.
Preferably, hardware abstraction should be done in a separate module altogether, so you never actually fiddle with registers directly in the application portion. By making code modular, porting code to a new platform becomes very easy. Want to use a new microcontroller? No problem, just replace/modify the hardware abstraction module and you're set.

Good code usually doesn't even need comments, because the names of the functions and variables themselves are descriptive enough to make it clear what something is doing.

[nuclearcat]

I think when device is released and if it will be affordable, we might see competition who can (IF CAN!) write better code (and hopefully giving away to Dave)

Probably but I've never really understood working for free, seems like a poor business model. 

You can gain a lot from writing free code.
)Community recognition. When you contribute to opensource, your potential employer will probably get an positive impression about you even before he looks at your CV.
In some cases, if you are contractor - you may be hired, since such portfolio is the best portfolio.
)Honing your skills and you might get free beta-testing and free review by professionals, including (probably) Dave. It is unlikely that you will have enough money to make Dave interested to touch and look to your proprietary code otherwise.
)Competition. Its FUN to be best!

And not only about gaining something. Altruism is good for humanity. Starting from the fact that the forum(software running it, OS, etc) we are talking in - exists, because many programmers once gifted the world pieces of their code.

[nuclearcat]

As soon as software becomes slightly more complex, you need to start splitting it in logical units, which you put into separate files. A well written software should not have files that stretch for more than a couple of hundred lines. There are of course always exceptions to this rule, but in general, one shouldn't have to scroll around and search for a particular part of code.

Next thing is abstraction. Just tossing register names around all over the place is very poor form. At the very least they should be macroed away with descriptive names.
Preferably, hardware abstraction should be done in a separate module altogether, so you never actually fiddle with registers directly in the application portion. By making code modular, porting code to a new platform becomes very easy. Want to use a new microcontroller? No problem, just replace/modify the hardware abstraction module and you're set.

Good code usually doesn't even need comments, because the names of the functions and variables themselves are descriptive enough to make it clear what something is doing.

Anonymous function (refered as authored by klingon) probably overkill for embedded and exactly opposite "of easy to understand code".
And whole fact returning pointer to *char by anonymous function might be fine on PC, but ehm, i think questionable in embedded.
But thats IMHO, i might be wrong.
I generally try to be careful and conservative with C++ features in embedded, you never know which function will make your code "suddenly overweight" or will start using "under the hood" dynamic memory allocation/reentrant functions/etc and guarantee you problems.
But it’s best to prove that, if i can write better code. Sure if i will be alive by the time uSupply will be released for sales.

[splin]

Good code usually doesn't even need comments, because the names of the functions and variables themselves are descriptive enough to make it clear what something is doing.

int Check_DMA_NotAboutToAcessSPI_CMD_RegisterWhilstUART1_TX_CompleteFlagSet_ToAvoidLockup_DescribedInErrata3.16.7(void);

[ataradov]

There is obviously a need for things like erratas and minor notes. There is no need to write poems, or a typical copy-paste with boiler-plate description of each parameter.

If the code implements some non-standard algorithm, the comments may help too. Especially on a way some constants are derived and stuff like that.

Peripheral drivers rarely need comments. In most cases the first thing I do if I really need to understand some code, I remove all the comments.

[ehughes]

I have been wanting to experiment with cmake and I noticed that Dave had a nice little template for getting started.


Some of my longer term projects could benefit from a more (easily) configurable build system.

[EEVblog]

I think when device is released and if it will be affordable, we might see competition who can (IF CAN!) write better code (and hopefully giving away to Dave)

Probably but I've never really understood working for free, seems like a poor business model. 

You can gain a lot from writing free code.
)Community recognition. When you contribute to opensource, your potential employer will probably get an positive impression about you even before he looks at your CV.
In some cases, if you are contractor - you may be hired, since such portfolio is the best portfolio.
)Honing your skills and you might get free beta-testing and free review by professionals, including (probably) Dave. It is unlikely that you will have enough money to make Dave interested to touch and look to your proprietary code otherwise.
)Competition. Its FUN to be best!
And not only about gaining something. Altruism is good for humanity. Starting from the fact that the forum(software running it, OS, etc) we are talking in - exists, because many programmers once gifted the world pieces of their code.

I had someone charge most of my uWatch code to include playing chess and other cool stuff, and he even write a manual for it:
All off his own bat, because he just wanted to, and bragging rights on the hpmusuem forum tool I guess. A true calculator nerd.
http://www.taswegian.com/uwatch/uWatch.pdf

[EEVblog]

Anonymous function (refered as authored by klingon) probably overkill for embedded and exactly opposite "of easy to understand code".
And whole fact returning pointer to *char by anonymous function might be fine on PC, but ehm, i think questionable in embedded.
But thats IMHO, i might be wrong.
I generally try to be careful and conservative with C++ features in embedded, you never know which function will make your code "suddenly overweight" or will start using "under the hood" dynamic memory allocation/reentrant functions/etc and guarantee you problems.
But it’s best to prove that, if i can write better code. Sure if i will be alive by the time uSupply will be released for sales.

The other thing is that David kept finding issues in the C++ compiler (and STM target), as in bugs or quirks that prevented the program from working, and in one case resulted in weeks worth of project delay until the compiler could be patched.
So yeah, the further you push the obscurities of the compiler, the more potential issues you would have for various reasons.

[nuclearcat]

The other thing is that David kept finding issues in the C++ compiler (and STM target), as in bugs or quirks that prevented the program from working, and in one case resulted in weeks worth of project delay until the compiler could be patched.
So yeah, the further you push the obscurities of the compiler, the more potential issues you would have for various reasons.

This is most common problem for those programmers who take for granted fancy features of c++17/etc and worked with them mostly on non-embedded targets.
I hit my hands when i try to use "on autopilot" convenient  std::vector(for example) in embedded, after got used to it hour ago on some linux software.
Nevertheless, the basic conveniences of C++ in the form of classes, constructors and destructors are reliable, thanks to the army of arduino testers (gcc/g++ under hood). And they make life much simpler and code more readable even for 8-bit AVR.

[EEVblog]

The other thing is that David kept finding issues in the C++ compiler (and STM target), as in bugs or quirks that prevented the program from working, and in one case resulted in weeks worth of project delay until the compiler could be patched.
So yeah, the further you push the obscurities of the compiler, the more potential issues you would have for various reasons.

This is most common problem for those programmers who take for granted fancy features of c++17/etc and worked with them mostly on non-embedded targets.

Yes, I don't think David had issue with them on his PC stuff.

[hans]

As soon as software becomes slightly more complex, you need to start splitting it in logical units, which you put into separate files. A well written software should not have files that stretch for more than a couple of hundred lines. There are of course always exceptions to this rule, but in general, one shouldn't have to scroll around and search for a particular part of code.

Next thing is abstraction. Just tossing register names around all over the place is very poor form. At the very least they should be macroed away with descriptive names.
Preferably, hardware abstraction should be done in a separate module altogether, so you never actually fiddle with registers directly in the application portion. By making code modular, porting code to a new platform becomes very easy. Want to use a new microcontroller? No problem, just replace/modify the hardware abstraction module and you're set.

Good code usually doesn't even need comments, because the names of the functions and variables themselves are descriptive enough to make it clear what something is doing.

Anonymous function (refered as authored by klingon) probably overkill for embedded and exactly opposite "of easy to understand code".
And whole fact returning pointer to *char by anonymous function might be fine on PC, but ehm, i think questionable in embedded.
But thats IMHO, i might be wrong.
I generally try to be careful and conservative with C++ features in embedded, you never know which function will make your code "suddenly overweight" or will start using "under the hood" dynamic memory allocation/reentrant functions/etc and guarantee you problems.
But it’s best to prove that, if i can write better code. Sure if i will be alive by the time uSupply will be released for sales.

Anonymous functions are sometimes useful, but often not in embedded.

C++ templates for embedded are useful, but also to certain degree. I like the tidiness of the codebase, it seems well maintained and structured. However, I've seen a few cases where I don't think I would have used a template, as the code size/speed gain is probably very small or absent (assuming the optimizer is turned on sufficiently).

Being careful with mechanic/function selection of C++ is a good point. You need to be aware of how the STL functions are implemented, and which are suitable for use on embedded, and also at which portions of the code, or are better left for unit tests (e.g. if the library uses dynamic memory allocation such as std::vector vs std:array). However, I think there are many language features/gems that are too good to leave untouched, for example templates and fast reusage of existing code using classes.

[NivagSwerdna]

You can gain a lot from writing free code.
)Community recognition. When you contribute to opensource, your potential employer will probably get an positive impression about you even before he looks at your CV.
In some cases, if you are contractor - you may be hired, since such portfolio is the best portfolio.
)Honing your skills and you might get free beta-testing and free review by professionals, including (probably) Dave. It is unlikely that you will have enough money to make Dave interested to touch and look to your proprietary code otherwise.
)Competition. Its FUN to be best!

And not only about gaining something. Altruism is good for humanity. Starting from the fact that the forum(software running it, OS, etc) we are talking in - exists, because many programmers once gifted the world pieces of their code.

I actually contribute to at least three open source projects.... but I still think it is a rubbish business model.  [I'm probably jaded by having spent decades contracting]