MugenSAS / osc-cpp-qt

OSC (Open Sound Control) for C++ using Qt (Mugen implementation)
MIT License
28 stars 3 forks source link

Other than using Qt classes for data, this seems to not create QObject classes for Signals and Slots #2

Closed themolecule closed 8 years ago

themolecule commented 8 years ago

Unless I'm missing it, which is totally possible, there doesn't seem to be an object that will emit signals or connect to them.

Having trouble seeing how to connect to these classes using the test suite... gtest.h seems to be deprecated... but again I may be missing something.

themolecule commented 8 years ago

I get it... signals and slots wouldn't make sense in this context.

but an example using Qt's network classes would be helpful... or a subclass of QTcpSocket specifically for this implementation would be helpful, which could emit interesting things.

druidPollux commented 8 years ago

About gTest, you can find supported repository here: https://github.com/google/googletest.git The library does not provide comunication layer, it is just about writing and reading message.

themolecule commented 8 years ago

ok... I'll write something and if you like it we can add it to the repo

druidPollux commented 8 years ago

No problem, feel free to enhance it.

themolecule commented 8 years ago

seems like OscMessageComposer needs a copy constructor

themolecule commented 8 years ago

The way you have pointers working, a copy constructor is not easy...

With all due respect, you're not using pointers properly, and you should know that QByteArray is very clever about maintaining its private data across copies, so data is only duplicated at the right time.

You're using an mOwner property in ByteBuffer.h to determine if memory should be released in the destructor, but Qt has already worked out a lot of that stuff if you work with parent objects, copy constructors, etc.

The QVector you have in OscBundleComposer.h uses pointers, which is Ok I guess, but because of your subclass structure it's not possible to use objects instead of pointers to capture the idea, I think.

It's maybe right-ish, but I think you are having memory leaks all over the place. You may want to run valgrind to see what's happening. Again, I may be totally wrong and you've worked all this out. I am at times guilty of the same structures but I always stress out to make sure memory is not leaking.

It's entirely possible that you are more clever than me about pointers, but you should at least provide a class that will support something simple like:

//osc_socket.h

#ifndef OSC_SOCKET_H
#define OSC_SOCKET_H

#include <QHostAddress>
#include <QAbstractSocket>

#include "osc/composer/OscMessageComposer.h"

class osc_socket : public QObject
{
    Q_OBJECT

    public:
        explicit osc_socket(QAbstractSocket::SocketType type, QHostAddress host, quint16 port, QObject *parent = 0);
        virtual ~osc_socket();

    private:
        QAbstractSocket::SocketType _type;
        QAbstractSocket             *_socket;
        QHostAddress                _host;
        quint16                     _port;
        int                         _counter;

        void timerEvent(QTimerEvent *t);
        void sendMessage(OscMessageComposer *message);      //works
        void sendMessage(OscMessageComposer message);       //crashes because of (lack of) copy constructor

    signals:

    public slots:
        void readPendingDatagrams();

};

#endif // OSC_SOCKET_H

...

//osc_socket.cpp

#include <QTcpSocket>
#include <QUdpSocket>

#include "osc/composer/OscMessageComposer.h"

#include "osc_socket.h"

osc_socket::osc_socket(QAbstractSocket::SocketType type, QHostAddress host, quint16 port, QObject *parent)
: QObject(parent)
, _type(type)
, _socket(0)
, _host(host)
, _port(port)
{
    switch(type)
    {
        case QAbstractSocket::TcpSocket:            _socket = new QTcpSocket(this);
                                                    printf("Created OSC TCP socket on %s:%d\n",qPrintable(_host.toString()),_port);
                                                    break;

        case QAbstractSocket::UdpSocket:            _socket = new QUdpSocket(this);
                                                    printf("Created OSC UDP socket on %s:%d\n",qPrintable(_host.toString()),_port);
                                                    break;

        case QAbstractSocket::UnknownSocketType:    throw;
    };

    if (_socket->bind(_host, _port))
        printf("Successfully bound port\n");
    else
        printf("Failed to bind port\n");

    //for later
    //connect(_socket, SIGNAL(readyRead()), this, SLOT(readPendingDatagrams()));

    //check for success
    _socket->connectToHost(_host,_port);

    startTimer(1000);
}

osc_socket::~osc_socket()
{
    //already happens because of parent relationship
    //delete _socket;
}

void osc_socket::timerEvent(QTimerEvent *)
{
    QString message = "Counter: "+QString::number(_counter);

    printf("%s\n",qPrintable(message));

    OscMessageComposer c(_host.toString());
    c.pushString(message);
    c.pushInt32(_counter);

    _counter++;

    sendMessage(&c);        //works
    //sendMessage(c);       //crashes because of destructor on local object

    //but is memory lost here?
}

//this works:
void osc_socket::sendMessage(OscMessageComposer *message)
{
    QByteArray *b = message->getBytes();
    _socket->write(*b);
}

//this crashes:
void osc_socket::sendMessage(OscMessageComposer message)
{
    QByteArray *b = message.getBytes();
    _socket->write(*b);
}

At the moment, I just need something that will work, so I'm moving forward, but I thought I would let you know what I've found.

getBytes should not return a pointer, as QByteArray already does that magic internally, and quite well.

Thanks for posting the code... It helps a lot!

themolecule commented 8 years ago

Here's what I came up with as an example:

//main.cpp
#include <QCoreApplication>

#include "osc_socket.h"

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    osc_socket s(QAbstractSocket::UdpSocket, QHostAddress::LocalHost, QString(argv[1]).toInt());

    return a.exec();
}
//osc_socket.h
#ifndef OSC_SOCKET_H
#define OSC_SOCKET_H

#include <QHostAddress>
#include <QAbstractSocket>

#include "osc/composer/OscMessageComposer.h"
#include "osc/reader/OscMessage.h"

class osc_socket : public QObject
{
    Q_OBJECT

    public:
        explicit osc_socket(QAbstractSocket::SocketType type, QHostAddress host, quint16 port, QObject *parent = 0);
        virtual ~osc_socket();

    private:
        QAbstractSocket::SocketType _type;
        QAbstractSocket             *_socket;
        QHostAddress                _host;
        quint16                     _port;
        int                         _counter;

        QByteArray                  _buffer;

        void timerEvent(QTimerEvent *t);
        void sendMessage(OscMessageComposer *message);

        void parseIncomingMessages();

    signals:
        void message(OscMessage *m);

    public slots:
        void readPendingUdpDatagrams();
        void readPendingTcpData();

};

#endif // OSC_SOCKET_H
//osc_socket.cpp

#include <QCoreApplication>
#include <QException>
#include <QTcpSocket>
#include <QUdpSocket>

#include "osc/composer/OscMessageComposer.h"
#include "osc/reader/OscReader.h"
#include "tools/ByteBuffer.h"

#include "osc_socket.h"

osc_socket::osc_socket(QAbstractSocket::SocketType type, QHostAddress host, quint16 port, QObject *parent)
: QObject(parent)
, _type(type)
, _socket(0)
, _host(host)
, _port(port)
{
    switch(type)
    {
        case QAbstractSocket::TcpSocket:            _socket = new QTcpSocket(this);
                                                    printf("Created OSC TCP socket on %s:%d\n",qPrintable(_host.toString()),_port);
                                                    connect(_socket, SIGNAL(readyRead()), this, SLOT(readPendingTcpData()));
                                                    break;

        case QAbstractSocket::UdpSocket:            _socket = new QUdpSocket(this);
                                                    printf("Created OSC UDP socket on %s:%d\n",qPrintable(_host.toString()),_port);
                                                    connect(_socket, SIGNAL(readyRead()), this, SLOT(readPendingUdpDatagrams()));
                                                    break;

        case QAbstractSocket::UnknownSocketType:    throw;
    };

    /*
    if (_socket->bind(_host, _port))
        printf("Successfully bound port\n");
    else
        printf("Failed to bind port\n");
    */

    _socket->connectToHost(_host,_port);

    startTimer(100);
}

osc_socket::~osc_socket()
{
    delete _socket;
}

void osc_socket::timerEvent(QTimerEvent *)
{
    if (_counter == 1500)
    {
        QCoreApplication::quit();
        return;
    }

    if (_counter < 1500)
    {
        QString path = QString("/some/%1/path").arg(_counter);
        QString message = "Counter: "+QString::number(_counter);

        printf("%s %s\n",qPrintable(path),qPrintable(message));

        OscMessageComposer c(path);
        c.pushString(message);
        c.pushInt32(_counter);

        sendMessage(&c);
    }

    _counter++;
}

void osc_socket::sendMessage(OscMessageComposer *message)
{
    QByteArray *b = message->getBytes();

    _socket->write(*b);

    delete b;
}

void osc_socket::readPendingUdpDatagrams()
{
    QUdpSocket *udp = qobject_cast<QUdpSocket *>(_socket);

    if (udp)
    {   while (udp->hasPendingDatagrams())
        {
            //printf("received message!\n");

            QByteArray datagram;
            datagram.resize(udp->pendingDatagramSize());
            QHostAddress sender;    //unused, but maybe useful
            quint16 senderPort;     //unused, but maybe useful

            udp->readDatagram(datagram.data(), datagram.size(), &sender, &senderPort);
            _buffer += datagram;

            parseIncomingMessages();
        }
    }
    else
        throw;
}

void osc_socket::readPendingTcpData()
{
    QTcpSocket *tcp = qobject_cast<QTcpSocket *>(_socket);

    if (tcp)
    {
        _buffer += tcp->readAll();

        parseIncomingMessages();
    }
    else
        throw;
}

void osc_socket::parseIncomingMessages()
{
    try
    {
        //use a while loop to clear the buffer... more than one message may be in there, or an incomplete one.
        while (_buffer.size())
        {
            OscReader r(&_buffer);
            OscMessage *m = r.getMessage();
            qint32 p = m->getPacket()->getPosition();

            if (m)
            {
                printf("  received message:\n");
                printf("   %s\n",qPrintable(m->getAddress()));
                int n = m->getNumValues();
                for (int i=0;i<n;i++)
                {
                    OscValue *v = m->getValue(i);
                    switch(v->getTag())
                    {
                        case 's':       printf("   %s\n",qPrintable(v->toString()));    break;
                        case 'i':       printf("   %d\n",v->toInteger());               break;
                        default:        printf("   tag: %c\n",v->getTag());             break;
                    }
                }

                //_buffer.clear();
                _buffer.remove(0,p);

                emit message(m);
                delete m;
            }
            //else
            //  exception was thrown
        }
    }
    catch(const QException &e)
    {
        printf("OSC_Socket Message exception: %s\n",e.what());
    }
    catch(...)
    {
        printf("OSC_Socket Incomplete or mal-formed message...\n");
    }
}

...and a simple echo server for OSC on UDP:

/* 
 * udpserver.c - A simple UDP echo server 
 * usage: udpserver <port>
 */

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <netdb.h>
#include <sys/types.h> 
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>

#define BUFSIZE 4024

/*
 * error - wrapper for perror
 */
void error(char *msg) {
  perror(msg);
  exit(1);
}

int main(int argc, char **argv) {
  int sockfd; /* socket */
  int portno; /* port to listen on */
  unsigned int clientlen; /* byte size of client's address */
  struct sockaddr_in serveraddr; /* server's addr */
  struct sockaddr_in clientaddr; /* client addr */
  struct hostent *hostp; /* client host info */
  char buf[BUFSIZE]; /* message buf */
  char *hostaddrp; /* dotted decimal host addr string */
  int optval; /* flag value for setsockopt */
  int n; /* message byte size */

  /* 
   * check command line arguments 
   */
  if (argc != 2) {
    fprintf(stderr, "usage: %s <port>\n", argv[0]);
    exit(1);
  }
  portno = atoi(argv[1]);

  /* 
   * socket: create the parent socket 
   */
  sockfd = socket(AF_INET, SOCK_DGRAM, 0);
  if (sockfd < 0) 
    error("ERROR opening socket");

  /* setsockopt: Handy debugging trick that lets 
   * us rerun the server immediately after we kill it; 
   * otherwise we have to wait about 20 secs. 
   * Eliminates "ERROR on binding: Address already in use" error. 
   */
  optval = 1;
  setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, 
         (const void *)&optval , sizeof(int));

  /*
   * build the server's Internet address
   */
  bzero((char *) &serveraddr, sizeof(serveraddr));
  serveraddr.sin_family = AF_INET;
  serveraddr.sin_addr.s_addr = htonl(INADDR_ANY);
  serveraddr.sin_port = htons((unsigned short)portno);

  /* 
   * bind: associate the parent socket with a port 
   */
  if (bind(sockfd, (struct sockaddr *) &serveraddr, 
       sizeof(serveraddr)) < 0) 
    error("ERROR on binding");

  /* 
   * main loop: wait for a datagram, then echo it
   */
  clientlen = sizeof(clientaddr);
  while (1) {

    /*
     * recvfrom: receive a UDP datagram from a client
     */
    bzero(buf, BUFSIZE);
    n = recvfrom(sockfd, buf, BUFSIZE, 0,
         (struct sockaddr *) &clientaddr, &clientlen);
    if (n < 0)
      error("ERROR in recvfrom");

    /* 
     * gethostbyaddr: determine who sent the datagram
     */
    hostp = gethostbyaddr((const char *)&clientaddr.sin_addr.s_addr, 
              sizeof(clientaddr.sin_addr.s_addr), AF_INET);
    if (hostp == NULL)
      error("ERROR on gethostbyaddr");
    hostaddrp = inet_ntoa(clientaddr.sin_addr);
    if (hostaddrp == NULL)
      error("ERROR on inet_ntoa\n");
    printf("server received datagram from %s (%s)\n", 
       hostp->h_name, hostaddrp);
    printf("server received %ld/%d bytes: %s\n", strlen(buf), n, buf);

    /* 
     * sendto: echo the input back to the client 
     */
    //n = sendto(sockfd, buf, strlen(buf), 0, 
    n = sendto(sockfd, buf, (size_t) n, 0, 
           (struct sockaddr *) &clientaddr, clientlen);
    if (n < 0) 
      error("ERROR in sendto");
  }
}

Hopefully this helps someone getting started!