LINUX.ORG.RU

Конвертация 3 байт в int, я правильно делаю?

 , ,


0

1

В спеках пакетов с данными от некоторой железки написано «Первые 8 байт отбрасываются, а далее идёт 150 чисел. Числа записаны как беззнаковые целые по 3 беззнаковых байта в big endian (старший, средний, младший). Значение отсчёта умножаем на цену деления (2.5v/0xFFFFFF) и вычитаем середину шкалы (1.25v)». Я получаю массив char'ов (QByteArray) и далее для получения всех чисел делаю следующее:

static double SomeClass::getOneReading(QByteArray::ConstIterator &it)
{
    const unsigned char b1 = *(it++);
    const unsigned char b2 = *(it++);
    const unsigned char b3 = *(it++);

    // Не преобразованное показание АЦП
    const quint32 block =
#if Q_BYTE_ORDER == Q_BIG_ENDIAN
            (b1 << 24) +
            (b2 << 16) +
            (b3 << 8);
#else
            b3 +
            (b2 << 8) +
            (b1 << 16);
#endif

    /// Цена деления (коэффициент перевода показаний АЦП в напряжение)
    static const double delta = 2.5 / 0xFFFFFF;
    /// Середина шкалы
    static const double Vcm = 1.25;

    return double(block) * delta - Vcm;
}

void SomeClass::processRecord(const QByteArray &recordDatagram)
{
    QList<double> readings;
    readings.reserve(150);

    // Первые 8 байт нас интересуют
    QByteArray::ConstIterator it = recordDatagram.constBegin();
    for (int i = 0; i < 8; ++i) {
        ++it;
    }

    // Нам интересны 450 байт из пакета с данными
    QByteArray::ConstIterator end = it;
    for (int i = 0; i < 450; ++i) {
        ++end;
    }

    while (it != end) {
        Q_ASSERT(it < end);
        readings << getOneReading(it);
    }

    emit gotRecord(readings);
}

Здесь всё правильно, или же вкралась некая ошибка? Я спрашиваю потому, что производитель железки, данные с которой я обрабатываю, катит на меня бочку, что я, мол, неправильно обрабатываю данные, а потому с некоторой периодичностью получается чушь вместо правильных данных.

Косяк заключается в том, что раз в N пакетов (N~=20-30) вместо нормальных данных получается какой-то непонятный забор, будто от переполнения буфера.

Я почти уверен, что проблема не в моём коде, а в железке, но вдруг ЛОР заметит здесь некий невидимый моим привыкшим к высокоуровневому программированию глазам косяк?

★★★★★

Последнее исправление: Obey-Kun (всего исправлений: 2)

не полностью смотрел, но у тебя точно в буфер передаваемый в processRecord массив целиком содержит сообщение от аппаратуры? Дробления (aka фрагментации) случаем нету?

Atlant ★★★★★
()
Ответ на: комментарий от Atlant

получаешь данные не по COM-порту? тормозов системы нету?

Atlant ★★★★★
()
for (int i = 8; i < 459; i += 3)
{
    const quint32 block =
#if Q_BYTE_ORDER == Q_BIG_ENDIAN
            (recordDatagram.at(i) << 24) +
            (recordDatagram.at(i+1) << 16) +
            (recordDatagram.at(i+2) << 8);
#else
            (recordDatagram.at(i+2) +
            (recordDatagram.at(i+1) << 8) +
            (recordDatagram.at(i) << 16);
#endif
    readings << block * 2.5 / 0xFFFFFF - 1.25;
}

Больно было смотреть на порнографию с указателями.

devpony
()
Ответ на: комментарий от amphibrakhij

Косяк заключается в том, что раз в N пакетов (N~=20-30) вместо нормальных данных получается какой-то непонятный забор, будто от переполнения буфера.

Если я прав, и используется именно тот код, то данные лишь кажутся правильными, а ТС - невнимательный быдлокодер

amphibrakhij
()
    // Первые 8 байт нас интересуют
    QByteArray::ConstIterator it = recordDatagram.constBegin();
    for (int i = 0; i < 8; ++i) {
        ++it;
    }

    // Нам интересны 450 байт из пакета с данными
    QByteArray::ConstIterator end = it;
    for (int i = 0; i < 450; ++i) {
        ++end;
    }

По пальцам считать, несомненно, интересно, но нужен ли тут весь подобный обвес в принципе, если на традиционных указателях все намного проще:

for (it=data+8, end=data+458; it<end; it+=3) {
  res = double(*it<<16 + *(it+1)<<8 + *(it+2)) 
    * 2.5 / 0xfffffff - 1.25;
  ...
}

bormant ★★★★★
()
Последнее исправление: bormant (всего исправлений: 1)
Ответ на: комментарий от Gvidon

Как ты char на 24 бита сдвигаешь? Куда?

В int. Это хранится char, а в выражении уже значение типа int (signet/unsigned), нет?

bormant ★★★★★
()
Ответ на: комментарий от bormant

Да, был неправ. Перед сдвигом char повышается до инта

Gvidon ★★★★
()
Ответ на: комментарий от bormant

... ах да, pascal попутал с приоритетом операций, скобочки нужны вокруг <<, да и синтаксис массива нагляднее:

  res = double((it[0]<<16) + (it[1]<<8) + it[2])
    * 2.5 / 0xfffffff - 1.25;

bormant ★★★★★
()
Последнее исправление: bormant (всего исправлений: 1)

а к чему такая морока если «readings << getOneReading(it);» отбросит мучительно высчитанную дробную часть ?

MKuznetsov ★★★★★
()

Скорее всего проблема в формировании запроса / чтении ответа. Так что можно не смотря на алгоритм сохранить в файл recordDatagramm и посмотреть на содержимое (есть ли там «заборные» данные).

По коду. random access iteratirs поддерживают 1( n) продвижение. Так что можно писать auto it = data.begin() + 8, а не городить цикл. Ещё лучше использовать std::advence, в qt должен быть аналог.

Почему функция расчёта возвращает double ?

Макросы не нужны. Можно обойтись и без этого высера.

anonymous
()
Ответ на: комментарий от MKuznetsov

а к чему такая морока если «readings << getOneReading(it);» отбросит мучительно высчитанную дробную часть ?

Когда правил код при копипасте, опечатался, readings — это список double. Просто хотел изначально показать без преобразования в вольтаж.

Obey-Kun ★★★★★
() автор топика
Последнее исправление: Obey-Kun (всего исправлений: 1)

У меня очень странные ошибки вылазили, а оказалось стек маловат.

anonymous
()

(b1 << 24) +

используй uint32_t ВЕЗДЕ. И используй логические операторы ВЕЗДЕ (+ — это арифметическая). И всё будет хорошо.

for (int i = 0; i < 450; ++i) { ++end;

что, в QByteArray иначе никак? Зачем же так извращаться-то? ☹

drBatty ★★
()
Ответ на: комментарий от drBatty

что, в QByteArray иначе никак? Зачем же так извращаться-то? ☹

Перепутал с каким-то другими итераторами Qt, в которых нельзя сделать it = begin() + 8.

используй uint32_t ВЕЗДЕ.

Покажи, что ты имеешь в виду, пожалуйста.

И используй логические операторы ВЕЗДЕ (+ — это арифметическая).

Тут разве есть разница между '|' и '+'?

Obey-Kun ★★★★★
() автор топика
Ответ на: комментарий от drBatty

что, в QByteArray иначе никак? Зачем же так извращаться-то? ☹

Оказывается, итераторы в QByteArray вообще лучше не использовать — https://bugreports.qt-project.org/browse/QTBUG-19997. Но я, конечно, сомневаюсь, что проблема в этом.

Obey-Kun ★★★★★
() автор топика
Последнее исправление: Obey-Kun (всего исправлений: 1)

Я почти уверен, что проблема не в моём коде, а в железке

Ну так добавь логирование. Пусть выдаёт исходные данные, если бред при расчете.

Это не у тебя по udp сейсмостанции данные шлют?

Просто есть один проектец древний - приблизительно с такой же схемой. Так вот иногда действительно прилетают бредовые данные. Но при этом и структура пакета неправильная, так что они просто отбрасываются. Машина с сервером в локалке с кучей оффтоп машин. Я всегда считал, что это от них такой флуд идёт - ведь udp без соединений работает.

ziemin ★★
()
Ответ на: комментарий от ziemin

Это не у тебя по udp сейсмостанции данные шлют?

Ога.

Просто есть один проектец древний - приблизительно с такой же схемой. Так вот иногда действительно прилетают бредовые данные. Но при этом и структура пакета неправильная, так что они просто отбрасываются. Машина с сервером в локалке с кучей оффтоп машин. Я всегда считал, что это от них такой флуд идёт - ведь udp без соединений работает.

Даже если всего один регистратор в сети, то иногда получается чушь.

Obey-Kun ★★★★★
() автор топика
Ответ на: комментарий от Obey-Kun

используй uint32_t ВЕЗДЕ.

Покажи, что ты имеешь в виду, пожалуйста.

выражение имеет тип как у своих значений. Если тип значений разный, выражение преобразуется к старшему типу. Проблема в том, КОГДА это происходит. Сдвигая char влево на 8 бит ты можешь вполне получить себе char, со значением 0. А можешь получить int со знаком (отрицательный) (потому-что константа 8 имеет тип int). Потому вместо 8 надо бы писать 8U, дабы компилятор понял, что это беззнаковое. И складывая числа плюсиком ты тоже можешь получить что-то не то, а именно влияние лишних битов на результат. Даже если их отбросить, возможна ошибка. Есть операция OR '|' для этого.

Здесь у тебя вроде всё в порядке, но вообще - я-бы не стал так рисковать.

Тут разве есть разница между '|' и '+'?

именно здесь вроде-бы нет, если у тебя int на 4 байта, и вообще x86. Как оно взлетит в другом случае - я не знаю. Тебе нужны проблемы? Ну раз нужны — пиши так.

drBatty ★★
()
Ответ на: комментарий от nerdogeek

Да, сегодня я буду делать логгирование бинарных данных, просто хотел посмотреть, увидит ли ЛОР косяк в коде.

Obey-Kun ★★★★★
() автор топика
Ответ на: комментарий от Obey-Kun

Оказывается, итераторы в QByteArray вообще лучше не использовать — https://bugreports.qt-project.org/browse/QTBUG-19997. Но я, конечно, сомневаюсь, что проблема в этом.

а зачем тут вообще итераторы Qt? Нельзя было всё в простой массив засунуть?

drBatty ★★
()
Ответ на: комментарий от drBatty

То есть вместо

    const unsigned char b1 = *(it++);
    const unsigned char b2 = *(it++);
    const unsigned char b3 = *(it++);

    // Не преобразованное показание АЦП
    const quint32 block =
#if Q_BYTE_ORDER == Q_BIG_ENDIAN
            (b1 << 24) +
            (b2 << 16) +
            (b3 << 8);
#else
            b3 +
            (b2 << 8) +
            (b1 << 16);
#endif

будем делать

    const quint32 b1 = *(it++);
    const quint32 b2 = *(it++);
    const quint32 b3 = *(it++);

    // Не преобразованное показание АЦП
    const quint32 block =
#if Q_BYTE_ORDER == Q_BIG_ENDIAN
            (b1 << 24U) |
            (b2 << 16U) |
            (b3 << 8U);
#else
            b3 |
            (b2 << 8U) |
            (b1 << 16U);
#endif

Да?

Obey-Kun ★★★★★
() автор топика
Последнее исправление: Obey-Kun (всего исправлений: 1)

Работать это будет только на little-endian машине. Если ерунду для случая Q_BIG_ENDIAN убрать, работать будет везде.

Deleted
()
Ответ на: комментарий от Obey-Kun

Да?

обычно делают так: http://en.wikipedia.org/wiki/Bit_field

что-то типа

union {
 struct {
  unsigned :8;
  unsigned b1 :8;
  unsigned b2 :8;
  unsigned b3 :8;
 }bb;
 unsigned ui;
}block;

block.ui = 0x123456;// загрузка всего числа
block.bb.b2;// байт №2

как-то так. давно такого не писал. ИМХО Qt и даже вообще C++ тут совсем не катит.

drBatty ★★
()
Ответ на: комментарий от Obey-Kun

В спеках пакетов с данными от некоторой железки написано «Первые 8 байт отбрасываются

А что в первых 8 байтах? Смотрел когда-нибудь? Может типа «SUCESS!» а в сбойных «DATALOSS»? :)

ziemin ★★
()
Ответ на: комментарий от Obey-Kun

*it — это char, а надо unsigned char.

Wut? Ну так приведи, кто мешает? Я к тому, что для такой задачи итераторы — жуткий оверхед, а в твоём случае ещё и быдлокод. Всё решается на простом массиве и счётчике.

devpony
()

Какой ужас. Си++, итераторы, классы, Q_BYTE_ORDER (!!!11), и поверх всего - не-слово Reading.

#include <stdint.h>

#define ALEN(a) (sizeof(a)/sizeof((a)[0]))

const double delta = 1.25/0xffffff;
const double Vcm = 1.25;

struct adc_sample { uint8_t parts[3]; };

struct adc_packet {
  uint8_t pad1[8];
  struct adc_sample samples[150];
};

static unsigned cvt1(struct adc_sample *n)
{
  /* никаких проверок Q_BYTE_ORDER, блеать */
  return (n->parts[0] << 16) | (n->parts[1] << 8) | n-> parts[2];
}

void convert_adc_packet(const struct adc_packet *pkt, double *numbers)
{
   int i;

   for (i = 0; i < ALEN(pkt->samples); i++)
     numbers[i] = cvt1(&pkt->samples[i])*delta - Vcm;
}

P.S. не пользуйся битовыми полями.

P.P.S. к перемежающейся проблеме код преобразования наверняка не имеет никакого отношения.

tailgunner ★★★★★
()
Последнее исправление: tailgunner (всего исправлений: 1)

выведи массив принятых данных с десятков измерений и посмотри что там, может какой-то байт херится или наоборот лишний

нельзя делать как ты - ты вслепую работаешь с принятыми данными, не производя обзор массы входных данных

вот это const QByteArray &recordDatagram сделать так: qDebug() << «record=» << recordDatagram.toHex(); и то что получилось давай нам сюда

и еще: ты ПЕРЕУСЛОЖНЯЕШЬ работу с QByteArray, какие-то итераторы прилепил, это ж тупо массив данных, просто смешно

выведи нам кучу данных с правильными значениями и там где ошибка - мы сравним

I-Love-Microsoft ★★★★★
()
Последнее исправление: I-Love-Microsoft (всего исправлений: 1)
Ответ на: комментарий от I-Love-Microsoft

выведи нам кучу данных с правильными значениями и там где ошибка - мы сравним

+1

А ещё лучше дамп всего пакета с заголовками. Может всё-таки сбойные это левые. От другого источника.

ziemin ★★
()
Ответ на: комментарий от tailgunner

Удваиваю этого оратора вместе с его вариантом решения.

nanoolinux ★★★★
()
Ответ на: комментарий от I-Love-Microsoft

не-слово Reading.

Почему это не-слово? Вполне себе слово. Переводится как отсчёт или как измерение.

Obey-Kun ★★★★★
() автор топика
Ответ на: комментарий от tailgunner

Си++, итераторы, классы

Я с сокетом UDP работаю через QUdpSocket (плюс sql, сигналы-слоты и пр.), так почему бы мне не воспользоваться его контейнером для байтовых полей?

Obey-Kun ★★★★★
() автор топика
Ответ на: комментарий от Obey-Kun

так почему бы мне не воспользоваться его контейнером для байтовых полей?

Ты правда не понимаешь, почему описание формата пакета в виде структуры лучше, чем продемонстрированное тобой байтоебство с индексами? Вон из профессии.

tailgunner ★★★★★
()
Последнее исправление: tailgunner (всего исправлений: 1)
Ответ на: комментарий от bormant

Вот кусок спека:

Числа идут по 3 беззнаковых байта в big endian (старший, средний, младший). Числа положительные беззнаковые. Преобразуется так: делаем из этих 3 байтов unsigned int (последний байт - 0x00). Значение отсчёта умножаем на цену деления (2.5v/0xFFFFFF) и вычитаем середину шкалы (1.25v).

Всё ещё считаешь, что код неверен?

upd: а, ты про часть для big-endian машин. Это я уже убрал, так что там сейчас это:

    const quint32 block = (b1 << 16U) | (b2 << 8U) | b3;

Obey-Kun ★★★★★
() автор топика
Последнее исправление: Obey-Kun (всего исправлений: 1)
Ответ на: комментарий от tailgunner

Понимаю, но пакеты у меня представлены в виде класса, наследованного от QByteArray, который общий на все ответы от железки, с которой я работаю. А там бывают не только пакеты с данными, но и прочие пакеты (самодиагностики и пр.), принцип работы с которыми разный.

Obey-Kun ★★★★★
() автор топика
Ответ на: комментарий от Obey-Kun

пакеты у меня представлены в виде класса, наследованного от QByteArray, который общий на все ответы от железки

Нужно добавить содомии к этому трэшу и угару.

tailgunner ★★★★★
()
Последнее исправление: tailgunner (всего исправлений: 1)
Ответ на: комментарий от tailgunner

В любом случае, я тут не за идеальным кодом. Софт писался на коленке в ужасной спешке, т.к., как оказалось, софт поставщика железки слишком крив.

Я буду вводить factory для разных типов ответов, который будет создавать классы разного типа, но пока не до этого. Т.е. будет в том числе и класс DataAnswer, который будет всё так же наследован от QByteArray (точнее, от класса Answer, наследованного от QByteArray), но будет иметь метод, возвращающий массив с содержащимися в ответе (конвертированными) данными.

Obey-Kun ★★★★★
() автор топика
Последнее исправление: Obey-Kun (всего исправлений: 1)
Ответ на: комментарий от Obey-Kun

В любом случае, я тут не за идеальным кодом.

Не всегда можно заставить работать кривую программу. И еще раз, если ты не понял: преобразование этих 3 байт в число не зависит от endianness машины.

Я буду вводить factory для разных типов ответов, который будет создавать классы разного типа

Отлично. Это как раз та содомия, которой не хватает.

tailgunner ★★★★★
()
Ответ на: комментарий от tailgunner

И еще раз, если ты не понял: преобразование этих 3 байт в число не зависит от endianness машины.

Я же уже написал выше, что понял и исправил.

Отлично. Это как раз та содомия, которой не хватает.

Ок. Как бы сделал ты? Вот как оно сейчас:

/// Смотрим на сокете ответы на команду @p command и обрабатываем их
Irkut::RequestResult Irkut::processPendingDatagrams(const IrkutCommand &command)
{
    QList <IrkutAnswer> answers;
    while (mSocket->hasPendingDatagrams()) {
        answers.append(IrkutAnswer(mSocket, mLastCommandDateTime));
    }
    foreach(const IrkutAnswer &answer, answers) {
        if (answer.address() != mIrkutAddress) {
            continue;
        } else {
            if (sizeFitsDeclared(answer)) {
                if (answer.size() != command.wantedAnswerSize()) {
                    return RR_BadAnswer;
                } else {
                    // Если это ответ на команду запроса данных записи, обрабатываем запись.
                    // Содержимое прочих ответов (пока) нас не интересует.
                    if (command == IrkutCommand::kSendRecord) {
                        processRecord(answer);
                    }
                }
                return RR_Ok;
            }
        }
    }
    return RR_NoAnswer;
}

Я повторюсь, код писался в адовой спешке, т.к. на горизонте мельтешила анальная кара от заказчика.

Obey-Kun ★★★★★
() автор топика
Ответ на: комментарий от Obey-Kun

Ну и до кучи

/// Шлём команду @p command и обрабатываем пришедшие ответы
Irkut::RequestResult Irkut::request(const IrkutCommand &command)
{
    if (mSocket->state() != QAbstractSocket::BoundState
            && mSocket->state() != QAbstractSocket::ConnectedState) {
        return RR_CantSend;
    }

    if (mSocket->hasPendingDatagrams()) {
        qWarning("WARNING: had pending datagrams before sending \"%s\" request!",
                 qPrintable(command.title()));
        return processPendingDatagrams(*mLastCommand);
    }
    if (!sendCommand(command)) {
        return RR_CantSend;
    } else {
        mLastCommand = &command;
        mSocket->waitForReadyRead(command.answerWaitTime());
        if (!mSocket->hasPendingDatagrams()) {
            return RR_NoAnswer;
        } else {
            return processPendingDatagrams(command);
        }
        // Не дождались ответа от нашего Иркута, а значит он или не подключен
        // к сети или (например) мы запросили checkline, а он пишет
        return RR_NoAnswer;
    }
}

Obey-Kun ★★★★★
() автор топика
Ответ на: комментарий от Obey-Kun

Вот как оно сейчас:

Это только маленький и маловажный кусочек того, что есть. Релевантны только 2 строки:

QList <IrkutAnswer> answers;

Это означает, что ты зачем-то натянул один класс на все ответы устройства (очень разные, как я подозреваю). Зачем ты это сделал? Я бы описал каждый пакет Си-структурой, а для списка пакетов использовал указатель на union всех пакетов (C style) или boost::variant указателей на пакеты. Ах да, и не использовал бы Qt ни для чего, кроме GUI.

IrkutAnswer(mSocket, mLastCommandDateTime)

Это означает, что у твоего мегакласса есть конструктор, читающий сокет и создающий объект из прочитанных данных. Опять же - зачем? Читаешь данные, проверяешь их, и, если проверка пройдена, делаешь из _области памяти_ объект (тупым преобразованием void * в указатель на нужный тип).

код писался в адовой спешке, т.к. на горизонте мельтешила анальная кара от заказчика.

Некоторые вещи должны делаться автоматически.

tailgunner ★★★★★
()
Последнее исправление: tailgunner (всего исправлений: 1)
Ответ на: комментарий от Obey-Kun

и не использовал бы Qt ни для чего, кроме GUI.

А сокеты?

А что сокеты? Никогда не использовал классы Qt для работы с ними.

А работа с БД?

Вспоминая тему Devel о работе с БД от тебя же, не уверен, что тебе так уж нужна работа с СУБД. Впрочем, если реально нужна работа с СУБД, причем разными - да, наверное, Qt - это вариант.

tailgunner ★★★★★
()
Ответ на: комментарий от tailgunner

Читаешь данные, проверяешь их, и, если проверка пройдена, делаешь из _области памяти_ объект (тупым преобразованием void * в указатель на нужный тип).

А если проверка данных бывает разной?

Obey-Kun ★★★★★
() автор топика
Ответ на: комментарий от tailgunner

А что сокеты? Никогда не использовал классы Qt для работы с ними.

Софт-то и под виндой должен работать, и в Linux. Или иы хочешь сказать, что в стандартных библиотеках сокеты кроссплатформенные?

Obey-Kun ★★★★★
() автор топика
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.