Karlo Obad

Important changes to forums and questions

All forums and questions are now archived. To start a new conversation or read the latest updates go to forums.mbed.com.

6 years, 3 months ago.  This question has been closed. Reason: Off Topic

Help with writing my first library

Hello to all, i am writing my first library and i did look at Writing a Library - Cookbook | Mbed but i still don t understand what i m doing wrong. My library function called "led" should turn on one of three LEDs connected to digital output pins, if i send 0 it lights up 1. led , if i send 1 it lights up 2. led and so on.

In the comment i have written all of the errors that i get, please help, Thank you in advance!

.h file

#ifndef midi_control
#define midi_control

#include "mbed.h"

 class midi_control { //declaration does not declare anything (it is a warning not an error)

public:

    midi_control(PinName led1,PinName led2,PinName led3); // "error expected a ;"

    void led(int x);
  
private:  
    DigitalOut _led1;
    DigitalOut _led2;
    DigitalOut _led3;
};

#endif

.cpp file

#include "mbed.h"
#include "midi_control.h"

midi_control::led(PinName led1,PinName led2,PinName led3) : _led1(led1),_led2(led2),_led3(led3)

void midi_control::led(int x) {
    
     if (x==0) {                                 
            _led1=1;
            _led3=0;
        }
        else if (x==1){
            _led2=1;
            _led1=0;
            }
        else{
            _led3=1;
            _led2=0;
            }

}

And here is main program :

#include "mbed.h"
#include "midi_control.h"
midi_control light(PTB8,PTB9,PTB10); // "expected a )"
 
int main() {
    light.led(1); // "expression must have class type"
}

Thank you all for your help! I figured it out with your help and help from friend, now it works as i wanted to. I am very tankful for your answers, good to see this is friendly and helpful community. Andy A the logic is good because function led in main program will always take int values from 0 to 2 (0,1,2) and then again from 0 to 2, it will never go from 0 to 2 or something like that, but i understand what you wanted to say. Once more, many thanks! :)

posted by Karlo Obad 26 Jan 2018

2 Answers

Andy A
6 years, 3 months ago.

Not related to the compile errors but I think your led() function has some logical flaws. e.g. if you have the pattern led(0);led(2); you will end up with LEDs 1 and 3 on. If you have led(1);led(2); you will get just LED 3 on.

I'm guessing this probably isn't correct, fortunately it's easy to fix, you just need to specify each LED in each situation. Also you can use a switch rather than a series of else if lines, it makes the code look a little cleaner e.g.

void midi_control::led(int x) {
  switch(x) {
    case 0:
      _led1=1;
      _led2=0;
      _led3=0;
      break;
   case 1:
      _led1=0;
      _led2=1;
      _led3=0;
      break;
   case 2:
      _led1=0;
      _led2=0;
      _led3=1;
      break;
   default: // any other value and they all go off.
      _led1=0;
      _led2=0;
      _led3=0;
      break;
  }
}

Or if you want to get clever you could use a BusOut rather than three DigitalOuts, a little less intuitive but your led() function becomes a single line for anything up to 32 leds. For 3 LEDs setting each one individually isn't that much of a pain but if you ever need more than 3 then this approach is far nicer.

 class midi_control {
 
[...] 
private:  
    BusOut _leds;
};

midi_control::led(PinName led1,PinName led2,PinName led3) : _leds(led1,led2,led3) {
_leds=0; // turn all LEDs off on startup
}

void midi_control::led(int x) {
_leds=1<<x;
}

Accepted Answer
Motoo Tanaka
6 years, 3 months ago.

Hi,

You defined "midi_control" to NOTHING at the beginning of the header.

Changing the 2 lines as

#ifndef _midi_control_
#define _midi_control_
...

will take care of the problem

Meantime, in the cpp file, line4 should be

midi_control::midi_control(...

And last not least, probably PTB8, PTB9, PTB10 will not work as PwmOut, so use pins which supports PwmOut or just use DigitalOut.

moto

Hi, This is my trial.

midi_control.h

#ifndef _midi_control_h_
#define _midi_control_h_
 
#include "mbed.h"
 
class midi_control { 
public:
    midi_control(PinName led1, PinName led2, PinName led3);
    ~midi_control() ;
    
    void led(int x);
  
private:  
    DigitalOut _led1;
    DigitalOut _led2;
    DigitalOut _led3;
};
 
#endif

Since LEDs here are negative logic, I assigned initial value as 1 to turn them off.

midi_control.cpp

#include "midi_control.h"
 
midi_control::midi_control(PinName led1,PinName led2,PinName led3) : 
_led1(led1, 1),_led2(led2, 1),_led3(led3, 1)
{
}
 
midi_control::~midi_control() 
{
}

void midi_control::led(int x) 
{    
     if (x==0) { /* trun on LED1 */         
        _led1=0;
        _led2=1;
        _led3=1;
    } else if (x==1){ /* turn on LED2 */
        _led1=1;
        _led2=0;
        _led3=1;
    } else if (x==2){ /* turn on LED3 */
        _led1=1;
        _led2=1;
        _led3=0;
    } else { /* turn off all LEDs */
        _led1=1 ;
        _led2=1 ;
        _led3=1 ;
    }
}

main.cpp

#include "mbed.h"
#include "midi_control.h"
// midi_control light(PTB8,PTB9,PTB10); // "expected a )"
/* for FRDM-KL25Z */
midi_control light(PTB18,PTB19,PTD1);
 
int main() {
    int i ;
    while(1) {
        light.led(i); 
        i = (i + 1) % 3 ; /* make i as 0, 1, 2, 0, 1, .. */
        wait(0.5) ;
    }
}

moto

posted by Motoo Tanaka 26 Jan 2018