[c++] Verbeteren communicatie tussen robot en pc

[pientertje]

Hallo,

Ik ben bezig met de code voor een robot, en heb een vraagje over hoe ik iets zo handig mogelijk kan aanpakken. Ik zal eerst even uitleggen hoe de situatie nu is: De robot is opgebouwd uit verschillende classes (elke sensor, verwerkingsunit en motor heeft een eigen class), al deze classes hebben allemaal verschillende getters en setters om informatie op te halen of instellingen aan te passen (denk aan doel aanpassen, maar ook finetuning van filters). Deze dingen moeten allemaal via de seriële poort benaderd kunnen worden, dus heb ik een communicatie class geschreven waarin door middel van een sscanf, een string compare en een if/elseif structuur de commando's uitgefilterd worden. Voor elk comando is er dus een apparte "if" die iets met een specifieke class doet. Ook is er nog een algemene status info opdracht die alle informatie in één keer doorstuurt.

Dit systeem werkt prima, alleen is het een vreselijk werk om te onderhouden. Er komen de hele tijd functies bij en oude worden aangepast. Als ik nu een functie toevoeg moet ik dat op vier plekken doen(in de code van de betreffende class, in de .h file van de class, in de communicatie class, en op de pc).

Is er een manier om dit handig te doen?

Klein stukje uit de communicatie class:

C++:

if (c1 == "s") {
                zender.printf("%d %d %d %f %f %d %f %f %d\n\r",roer->get(),zeil->get(),kompas->get(),gps->get().lat,gps->get().lon,route->getActive(),route->afstand_tot_doel(), route->koers_tot_doel(),zeilmanual);

            }

            else if (c1 == "roer") {
                if (c2 == "set") {
                    if (set) {
                        sscanf (c3.c_str(),"%d",&n);
                        roer->set(n);
                    }
                } else if (c2 == "get") {
                    if (!set) {
                        zender.printf("%d\n\r",roer->get());
                    }
                }
            } else if (c1 == "zeil") {
                if (c2 == "set") {
                    if (set) {
                        sscanf (c3.c_str(),"%d",&n);
                        zeil->set(n);
                    }
                } else if (c2 == "get") {
                    if (!set) {
                        zender.printf("%d\n\r",zeil->get());
                    }
                }

                else if (c2 == "auto") {
                    if (set) {
                        sscanf (c3.c_str(),"%d",&n);
                        if (n == 1) {
                            zeiltimer.attach(this,&Status::autoZeil,0.1);
                            zeilmanual = 0;
                        }
                        if (n == 0) {
                            zeiltimer.detach();
                            zeilmanual = 1;

                        }
                    }
                    if (!set) {
                        zender.printf("%d\n\r",zeilmanual);

                    }
                }

[Verwijderd]

Ik denk dat het een goed idee is om gebruik te maken van callbacks en een map.

C++:

#include <iostream>
#include <string>
#include <map>

typedef int(*callback_t)(void);
std::map<std::string, callback_t> callbacks;

int foo(void)
{
    // Doe dingen
    return 0;
}

void register (std::string command, callback_t callback)
{
    callbacks[command] = callback;
}

void process(std::string command)
{
    callback_t callback = callbacks[command];

    if (NULL != callback) callback();
}

int main (void) {
    register("foo", foo);
    process ("foo"); // Voert de functie foo() uit, er kan ook niets gebeuren als er geen callback geregistreerd is.

    return 0;
}

Op deze manier is het gemakkelijk om te onderhouden en, nog belangrijker misschien, modulair.

Daarnaast denk ik dat het misschien, als het niet té overdreven is, een soort van terminal te implementeren. Dit is niet zo heel moeilijk met C++ en is met het mechanisme hierboven makkelijk uit te breiden.

[ Voor 28% gewijzigd door Verwijderd op 07-10-2011 18:38 ]

[farlane]

Wat je eigenlijk mist is een "manager object" die het verband tussen een commando en een unit(sensor/motor etc ) kan leggen, en "handler objecten" die zich bij die manager kunnen registreren.

Voor elke "unit" kun je op die manier een handler object aanmaken die de commando's voor die unit kent en afhandelt.

Op die manier hoef je maar 1 class aan te passen ( de handler dus ) als een nieuw commando wordt geimplementeerd, of 1 handler class toe te voegen en te registreren als je een nieuwe unit maakt:

C++:

#include <iostream>
#include <string>
#include <list>

struct Command
{
    std::string Unit;
    std::string Cmd;
    std::string Data;
};

class CommandHandler
{
public:
    const std::string Unit;

public:
    CommandHandler( const std::string& unit ) : Unit( unit )
    {
    }

    virtual void CommandReceived( const Command& cmd ) = 0;
};

class CmdManager
{
private:
    std::list<CommandHandler*> handlers;

public:
    void Register( CommandHandler* handler )
    {
        handlers.push_back( handler );
    }

    void HandleCmd( const Command& cmd )
    {
        std::list<CommandHandler*>::iterator it;

        for( it = handlers.begin(); it != handlers.end(); ++it )
        {
            if( cmd.Unit == (*it)->Unit )
                (*it)->CommandReceived( cmd );
        }
    }

};

class RudderHandler : public CommandHandler
{
public:
    RudderHandler( ) : CommandHandler( "roer" )
    {
    }
    void CommandReceived( const Command& cmd )
    {
        std::cout << "RudderHandler " << cmd.Unit << " " <<
            cmd.Cmd << "\n";
;   }
};

class SailHandler : public CommandHandler
{
private:
    std::string sail;

public:
    SailHandler( const std::string& sail ) : CommandHandler( "zeil" ), sail( sail )
    {

    }
    void CommandReceived( const Command& cmd )
    {
        std::cout << "SailHandler " << sail << " " << cmd.Unit << " " <<
            cmd.Cmd << "\n";
    }
};

static bool ReceiveCommand( Command* cmd )
{
    bool r = true;

    static int test = 0;

    switch( test++ )
    {
        case 0:
            cmd->Unit = "roer";
            cmd->Cmd = "set";
            cmd->Data = "";
            break;
        case 1:
            cmd->Unit = "zeil";
            cmd->Cmd = "set";
            cmd->Data = "";
            break;
        default:
            r = false;
    }
    return r;
}

int main()
{
    CmdManager mgr;
    RudderHandler rudder;
    SailHandler sail1( "zeil1" );
    SailHandler sail2( "zeil2" );

    mgr.Register( &rudder );
    mgr.Register( &sail1 );
    mgr.Register( &sail2 );

    Command cmd;

    while( ReceiveCommand( &cmd ) )
        mgr.HandleCmd( cmd );

    return 0;
}

[ Voor 76% gewijzigd door farlane op 07-10-2011 20:14 ]

[Matis]

@TS, misschien dat het nu (toevallig) goed gaat, maar Strings (of char*) hoor je niet met elkaar te vergelijken middels ==.

C:

if (strcmp(c1,"s") == 0) { 
/* of */
if(c1.compare("s") == 0)

Verder zijn natuurlijk de suggesties van de andere posters een stuk netter dan jouw code nu.

[ Voor 6% gewijzigd door Matis op 07-10-2011 21:08 ]

[pientertje]

Heel erg bedankt!

Na een half uur de code bestudeerd en getest te hebben begrijp ik het geloof ik. De commandoafhandeling zet ik in de classes van de sensoren zelf, en deze wordt op het juiste moment aangeroepen door de CmdManager. Ik moet elke sensor dus welk registreren bij de cmdmanager, maar als ik bij een bestaande sensor een commando toevoeg hoef ik niet alles aan te passen. Klopt dat?

Ik heb ook nog een andere vraag hierover. Op de pc draait dus software die de actuele sensorinformatie weergeeft. Deze vraagt steeds alle informatie op. Omdat het teveel overhead geeft als ik voor elke sensor een commando moet sturen heb ik één commando gemaakt voor vrijwel alle informatie. Hoe zou ik dat nu het beste kunnen implementeren met het nieuwe systeem?

@Matis
c1 t/m c3 zijn van het type 'String' (std::string) dus dan mag het geloof ik wel.

[farlane]

pientertje schreef op vrijdag 07 oktober 2011 @ 21:12:
Na een half uur de code bestudeerd en getest te hebben begrijp ik het geloof ik. De commandoafhandeling zet ik in de classes van de sensoren zelf, en deze wordt op het juiste moment aangeroepen door de CmdManager.

Nee, niet bij de sensoren zelf, maar in een aparte "handler" class ( die misschien wel veel weet van een of meerdere sensoren ). De sensoren staan lost van de commandoafhandeling.

Ik heb ook nog een andere vraag hierover. Op de pc draait dus software die de actuele sensorinformatie weergeeft. Deze vraagt steeds alle informatie op. Omdat het teveel overhead geeft als ik voor elke sensor een commando moet sturen heb ik één commando gemaakt voor vrijwel alle informatie. Hoe zou ik dat nu het beste kunnen implementeren met het nieuwe systeem?

Je weet hierop het antwoord als je de gedachte loslaat dat een CommandHandler bij een sensor hoort (en het antwoord op de vorige vraag begrijpt)

[pientertje]

Dus ik zou dan mijn commandhandlers moeten indelen in bijvoorbeeld groepen zoals 'sensoren' en 'actuatoren' en 'route'? Maar hoe kan ik dan uivlooien in de 'sensorenhandler' van welke sensor ik informatie wil? Ik hoop niet met een case structuur, want dat maakt het aanpassen er niet makkelijker op. Als ik elke sensor zijn eigen handler geef en in de class van de sensor integreer heb ik daar geen last van.

Is het niet handiger als ik de commandhandlers geen toegang geef tot de seriële poort maar een string laat returnen? Dan kan ik het verzenden overlaten aan de commandmanager.

[Verwijderd]

farlane schreef op vrijdag 07 oktober 2011 @ 20:03:
Wat je eigenlijk mist is een "manager object" die het verband tussen een commando en een unit(sensor/motor etc ) kan leggen, en "handler objecten" die zich bij die manager kunnen registreren.

Voor elke "unit" kun je op die manier een handler object aanmaken die de commando's voor die unit kent en afhandelt.

Op die manier hoef je maar 1 class aan te passen ( de handler dus ) als een nieuw commando wordt geimplementeerd, of 1 handler class toe te voegen en te registreren als je een nieuwe unit maakt:

Dus eigenlijk wat ik zei, maar dan OO .

[farlane]

pientertje schreef op vrijdag 07 oktober 2011 @ 21:39:
Dus ik zou dan mijn commandhandlers moeten indelen in bijvoorbeeld groepen zoals 'sensoren' en 'actuatoren' en 'route'? Maar hoe kan ik dan uivlooien in de 'sensorenhandler' van welke sensor ik informatie wil? Ik hoop niet met een case structuur, want dat maakt het aanpassen er niet makkelijker op. Als ik elke sensor zijn eigen handler geef en in de class van de sensor integreer heb ik daar geen last van.

Zoals je ziet in mijn voorbeeld heb ik geen switch structuur, en ik kan in de handler class zelf bepalen welke commando's ik afhandel door een "unit" te kiezen. Die beslissing zou je zelfs van de commandmanager class naar de handlers zelf kunnen verplaatsen.
Ik ga er trouwens wel vanuit dat al je commando's een zelfde structuur hebben, zoals in je eigen voorbeeld. ( En als dat niet zo is zou het eigenlijk zo moeten zijn. )

Meestal, als je op meerdere plekken switch'ed op dezelfde voorwaarde met verschillende acties daaraan gekoppeld, is dat een teken dat deze te vervangen zijn door een klassenstructuur met bovenin pure virtual functies : wat men ook wel een interface noemt. ( google )

Mijn punt is eigenlijk dat de koppeling van je commando's naar je eigen objecten niet zo hard zou moeten zijn : door er een 'handler class' tussen te zetten bereik ik dat.
Je eigen probleem is daar een voorbeeld van : je moet ook een handler maken die bij alle sensoren etc kan om daar informatie vandaan te halen voor je 'gecombineerde' commando.

Is het niet handiger als ik de commandhandlers geen toegang geef tot de seriële poort maar een string laat returnen? Dan kan ik het verzenden overlaten aan de commandmanager.

Sowieso moeten de commandhandlers niets met een seriele poort van doen hebben. Kijk naar mijn testcode, die weet ook niets van een seriele poort en toch kan ik de code draaien en testen.
Zelfs de CommandManager zou niet (hard) aan een medium gebonden hoeven te zijn, maar is misschien voor jou op dit momen wel de beste plek om het neer te zetten.

Verwijderd schreef op vrijdag 07 oktober 2011 @ 21:53:
Dus eigenlijk wat ik zei, maar dan OO .

Klopt, er stond C++ in de topicstart dus ik vond het wel te verantwoorden om een OO achtig ding te maken

[JeromeB]

farlane schreef op vrijdag 07 oktober 2011 @ 23:46:

[...]

Klopt, er stond C++ in de topicstart dus ik vond het wel te verantwoorden om een OO achtig ding te maken

De manier van Akathos is in principe ook wel OO als je de function-pointers vervangt door function-objects (de handlers).

[H!GHGuY]

Matis schreef op vrijdag 07 oktober 2011 @ 21:07:
@TS, misschien dat het nu (toevallig) goed gaat, maar Strings (of char*) hoor je niet met elkaar te vergelijken middels ==.

C:

if (strcmp(c1,"s") == 0) { 
/* of */
if(c1.compare("s") == 0)

Verder zijn natuurlijk de suggesties van de andere posters een stuk netter dan jouw code nu.

Daar is helemaal niets mis mee:

C++:

class string
{
   implicit string(const char*) { /* maakt een copy */; }
   bool operator==(const std::string& rhs);
   // <snip>
};

Er is een impliciete conversie van const char* naar std::string en die wordt dan ook gebruikt wanneer je een string vergelijkt met een const char*.

[Matis]

Ach, ik ben maar een simpele GNU C ontwikkelaar

[ Voor 4% gewijzigd door Matis op 08-10-2011 21:12 ]

[H!GHGuY]

Matis schreef op zaterdag 08 oktober 2011 @ 21:12:
Ach, ik ben maar een simpele GNU C ontwikkelaar

Geen probleem, zo leren we allemaal bij

[pientertje]

Ik ben wat aan het programmeren geslagen. Het is me bijna gelukt om het principe van de maps toe te passen op classes, en dat lukt aardig. Ik loop nog vast op één detail:

C++:

#include <iostream> 
#include <string> 
#include <map> 

typedef void(*callback_t)(void);


class Roer{
     
    std::map<std::string, callback_t> method_callbacks;
    private:
        void link_method(std::string command, callback_t callback);
        
    public:
        Roer(void);
        void process_method(std::string);
        void foo(void);
};


std::map<std::string, Roer*> class_callbacks;








Roer::Roer(){
    link_method("foo", foo);    //<---- Dit gaat fout: bootje.cpp:25:24: error: no matching function for call to &#8216;Roer::link_method(const char [4], <unresolved overloaded function type>)&#8217;
                                //      bootje.cpp:12:8: note: candidate is: void Roer::link_method(std::string, void (*)())
        
}

void Roer::link_method(std::string command, callback_t callback) 
{ 
    method_callbacks[command] = callback; 
} 


void Roer::process_method(std::string command) 
{ 
    std::cout << "Roer! Command: " << command << std::endl;
    
    callback_t callback = method_callbacks[command]; 

    if (NULL != callback) 
        callback(); 

} 

void Roer::foo(void){
    std::cout << "Foo!" << std::endl;
}

void link_class(std::string command, Roer* callback) 
{ 
    class_callbacks[command] = callback; 
} 

void process_class(std::string command, std::string s) 
{ 
    Roer * callback = class_callbacks[command]; 

    if (NULL != callback) 
        callback->process_method(s);
    
} 


int main (void) {
    Roer roer; 
    std::string command = "foo";
    
    link_class("roer", &roer);    
    process_class("roer", command);
    

    
    
    return 0; 
}

Het lukt dus niet om de method te koppelen aan de map. Volgensmij is dit gewoon een syntaxprobleem, maar ik kom er echt niet uit.

[.oisyn]

De syntax is &Roer::foo, maar dan ben je er nog niet. Het type daarvan is namelijk een void (Roer::*)(), oftewel een pointer-to-member-function, wat wat anders is dan een pointer-to-function. Om een dergelijke functie aan te roepen heb je een class instance nodig waar je de methode op aanroept.

Voorbeeld:

C++:

struct S
{
    int m_value;

    S(int i) : m_value(i) { }
    int TimesTwo() { return m_value * 2; }
    void Square() { return m_value * m_value; }
};

int main()
{
    S a(4);
    S b(10);

    void (S::*ptr)() = &S::TimesTwo;
    std::cout << (a.*ptr)() << ", " << (b.*ptr)() << std::endl;   // 8, 20

    ptr = &S::Square;
    std::cout << (a.*ptr)() << ", " << (b.*ptr)() << std::endl;   // 16, 100
}

Zoals je ziet, 'ptr' wijst alleen naar de method, niet naar een specifieke instance. De aanroep doe je dmv de .* operator (of ->* in geval van een pointer naar een class instance).

Nou gaat je manier in dit specifieke geval wel werken, je wilt de method in process_method() immers altijd aanroepen op je 'this'. Als je callback_t wijzigt naar void (Roer::*)() en in je Roer::process_method() gewoon (this->*callback)() doet, dan zal het doen wat je wilt.

Maar mooier en praktischer is misschien om gewoon tr1::function of boost::function te gebruiken.

[ Voor 7% gewijzigd door .oisyn op 10-10-2011 17:27 ]

[pientertje]

Als je callback_t wijzigt naar void (Roer::*)()

Waar precies? Bij de typedef? Want dat lukt niet, want Roer is op dat moment nog niet gedefiniëerd, en callback_t later definiëren werkt ook niet omdat deze weer in de definitie van Roer staat.

[ Voor 14% gewijzigd door pientertje op 10-10-2011 17:58 ]

[.oisyn]

Klopt, maar waarom zou je callback_t überhaupt buiten Roer definieren dan?

[farlane]

pientertje schreef op maandag 10 oktober 2011 @ 17:04:
Ik ben wat aan het programmeren geslagen. Het is me bijna gelukt om het principe van de maps toe te passen op classes, en dat lukt aardig. Ik loop nog vast op één detail:
Het lukt dus niet om de method te koppelen aan de map. Volgensmij is dit gewoon een syntaxprobleem, maar ik kom er echt niet uit.

Wat is eigenlijk de reden dat je met een (niet zo'n) kale functiepointer wilt werken ipv gebruik maken van polymorfisme?

[pientertje]

Hoe zou ik dan moeten toepassen met polymorfisme?

Ik heb nu een werkende code die eigenlijk doet wat ik wil:

C++:

#include <iostream>
#include <sstream> 
#include <string> 
#include <map>
#include <vector>


template <typename T>
T StringToNumber (const std::string& Text )
{                                           
    std::stringstream ss(Text);
    T result;
    return ss >> result ? result : 0;
}

std::string get_nth(std::string s, unsigned int n){
    std::vector<std::string> l;
    while (!s.empty() && l.size() <= n){
        l.push_back(s.substr(0,s.find(" ")));
        s.erase(0,s.find(" "));
        if (!s.compare(0,1," "))
            s.erase(0,1);
        
    }
    
    return l[n];
}

class Base_Processor{   
    public: 
        virtual void process_method(std::string command, std::string &result) = 0;  
};


class Roer : public Base_Processor{
    
    typedef void (Roer::*ptr)(std::string, std::string &); 
    std::map<std::string, ptr> method_callbacks;
    
    int stand;
    
    void link_method(std::string method_name, ptr method_ptr){
        method_callbacks[method_name] = method_ptr;
    }       
        
    public:
        Roer(void);
        
        void process_method(std::string command, std::string &result){
            ptr callback = method_callbacks[get_nth(command,1)]; 

            if (NULL != callback) 
                (this->*callback)(command, result); 
        }
        
        
        void get(std::string, std::string &);
        void set(std::string, std::string &);
};

Roer::Roer(){
    stand = 50; 
    link_method("get", &Roer::get);
    link_method("set", &Roer::set);     
}
 

void Roer::get(std::string command, std::string &result){
    std::stringstream s;
    s << stand;
    result = s.str();
}

void Roer::set(std::string command, std::string &result){
    stand = StringToNumber<int>(get_nth(command,2));
}

std::map<std::string, Base_Processor*> class_callbacks;
void link_class(std::string class_name, Base_Processor* class_ptr) 
{ 
    class_callbacks[class_name] = class_ptr; 
} 

void process_class(std::string command, std::string &result) 
{ 
    Base_Processor* callback = class_callbacks[get_nth(command,0)]; 

    if (NULL != callback) 
        callback->process_method(command, result);
    
} 


int main (void) {
    Roer roer; 

    std::string result;
    
    link_class("roer", &roer);
        
    process_class("roer get", result);    
    std::cout << result << std::endl;   //50
    
    process_class("roer set 123", result); 
    
    process_class("roer get", result);    
    std::cout << result << std::endl;   //123
    

    
    
    return 0; 
}

Hebben jullie nog verbeterpuntjes?

[ Voor 30% gewijzigd door pientertje op 11-10-2011 19:11 ]

[Verwijderd]

Waarom heb je de processing "Roer" genoemd?

Die naam beslaat niet echt waar de klasse verantwoordelijk voor is. Ik zie namelijk ook een methode "process_method" erin staan...

[ Voor 60% gewijzigd door Verwijderd op 11-10-2011 01:30 ]

[farlane]

pientertje schreef op maandag 10 oktober 2011 @ 22:44:
Hoe zou ik dan moeten toepassen met polymorfisme?

Op een vergelijkbare manier als ik heb gedaan?

Ik heb nu een werkende code die eigenlijk doet wat ik wil:

Volgens mij maak je het veel te gek. Als je dynamisch je commando handler interface wilt kunnen uirbreiden kun je eventueel een Visitor pattern gebruiken, maar zelfs dat zou ik je op dit moment niet aanraden : je lijkt de dynamiek bis ins geht nicht mehr te willen doorvoeren.

[.oisyn]

Ook is het een beetje suf dat elke instantie van Roer zijn eigen map heeft en initialiseert, terwijl deze typisch gewoon gedeeld kan worden tussen alle instanties.

[Verwijderd]

Ik vind het toch niet echt de netste oplossing

Ik zou een basis klasse erbij doen. Dan kan je gewoon makkelijk een dispatcher implementeren. Maar misschien dat dat een beetje overdreven is voor dit project...

[pientertje]

Ik probeerde nu om het af te maken een Base class te maken waarvan alle andere sensoren ed. van afgeleid konden worden. Daar wilde ik dan de link_method en process_method inzetten. Maar dat lukt helaas niet omdat ik dan met pointers naar de base class zit waarmee ik geen nieuwe functies van de afgeleide classes kan aanroepen.

@.oisyn
Het is een beetje suf, maar ik zal waarschijnlijk van elke class nooit meer dan één of twee instanties maken.

@edit kleine update (zie vorige post) met een base class, nu alleen nog proberen de dispatcher naar "boven" te verplaatsen. Ik kom alleen vreselijk in de knoop met "typedef void (Roer::*ptr)(std::string, std::string &);" Hoe kan ik deze *ptr universeel maken voor alle afgeleide classes van een base class?

[ Voor 28% gewijzigd door pientertje op 11-10-2011 19:12 ]