День отладки

Сижу на работе, медленно втухаю в интренеты, медитируя на то как комплектуха ко мне едет. Тут звонит заказчик и говорит, что мол у них мною разработанные девайсы не работают. Да ладно? А почему я об этом узнаю только сейчас? А не когда я их вам сдал еще в мае? Кто кипятком ссал и говорил, что ему срочно срочно? Два месяца прошло, а все еще срочно? Впрочем, теперь, наверное, действительно срочно. А они только соизволили их смонтировать в установки и проверить.
Присылает видео, по видео пляшут показания датчика давления. Хорошо так пляшут, с нуля до 65 очков.

Хм. Собираю из говна и палок и недоделанных плат еще один экземпляр того же устройства. Прошиваю. Втыкаю вместо датчика резистор. Там токовая петля от 4 до 20мА и резистором можно вполне годно эмулировать датчик. Все работает. Глюков не вижу. Прошу привезти датчики. Привозит, втыкаю — пляшет. Хм. Может датчик говно? Беру швейцарский, прецизионный — вешаю, пляшет, но уже сильно сильно реже. Ставлю резистор — стоит ровно. Странно…

Начинаю думать. Пересчитал фильтр, заменил кондер — стало сильно лучше. Пляска случается, но глазом ее уже не видно, понять, что у датчика превышение показаний можно только по срабатыванию защиты. Смотрю осциллографом линию. Вроде тихо и ничего нет. Но откуда то же АЦП хавает большие значения. Иголки? Не похоже.

Мда. Лезу в код. 90% его писал не я и он достался в наследство. Благо написан был в целом неплохо, был понятен и легко модифицировался. В подсчете данных с АЦП я ничего не менял, я добавил только обработку одного канала. Что это может быть? Перехлест каналов АЦП? Такое у меня уже было, когда из-за ошибки в коде перепутались каналы АЦП и чтение было со сдвигом. Тут же вроде все ровно. Тем более с резистором все ок работает, напряжение по второму каналу тоже отлично замеряется.

Создаю отдельный бранч, делаю вывод всего что можно на экранчик, чтобы видеть в реалтайме как оно меняется. Замер тока почти эталонный. Стоит не шевелится. А вот давление скачет. Пересчитал коэффициенты. Добавил больше усреднения. Все равно сработка защиты. Хм… срабатывает оно на 65 очков. А почему вдруг 65? Случайность? А может переполнение?

Добавил в функцию запроса давления отладочную строку, которая записывает максимальное давление которое прошло через функцию. Т.е. все запросы которые выполняются сравниваются с предыдущими и берется максимальное значение. Точно. Давление, в чистом виде, выловилось как 65525. Переменная uint_16t. Характерно. А это уже очень похоже на результат переполнения. Опять смотрю в код:

uint16_t PressureSensorGetPressure(void)
{
	if (ADC_GetValue_ma_x100() < 400) return 0;
	uint32_t Temp = (ADC_GetValue_ma_x100() - 400) * (HC_PRESSURESENSOR_VPI / 16);
	return Temp;
}

Хм, вроде переполнения не должно быть. Значение заранее обрезается по 400 (4мА токовой петли, нижняя граница). Вот только...

uint16_t ADC_GetValue_ma_x100(void)
{
uint32_t Integrator = 0;

	for (uint8_t i = 0; i < cADC_INTEGRATOR; i++)
			{
			Integrator += MeasuredValues[i];
			}
	Integrator /= pADC_Calibrate_K2;

	Integrator *= pADC_Calibrate_K1;


return Integrator;
}

Раньше наполнение буфера делалось вручную, в главном цикле. А у меня с добавлением нового канала это было вынесено в прерывание. И после того, как были отсеяны значения ниже нуля шины, ниже 400, вычитание смещения (-400) могло вывзвать косяк, т.к. делалось это с повторным пересчетом буфера, а он мог измениться в прерывании. И возле нулевых значений, который давал подключенный датчик, не нагруженный на давление, это хаотично вызывало переполнение. А вот на резисторе ток был примерно в середине диапазона, примерно на 70 атмосфер и там в переполнение уйти не могло. Сделал одно чтение буфера и проблема исчезла. Бонусом стал отлаженный аналоговый тракт и повысившаяся точность измерний.

Запись опубликована в рубрике Мастерская. Добавьте в закладки постоянную ссылку.

18 комментариев: День отладки

  1. koc9ti говорит:

    Типичный TOCTOU рэйс

    • DI HALT говорит:

      Ага. Поэтому я всегда стараюсь делать использование вывода функции через Temp если оно множественно используется.

  2. Ghiotto говорит:

    рэйс рэйсом но вообще вызывать дважды «тяжелую» функцию для получения одного значения, да еще и рядом — это должно быть запрещено в подкорке :-) впрочем, многие вещи туда попадают пока сам по граблям не походил

    • DI HALT говорит:

      Есть там местами такая хренота. Вроде бы код написан хорошо, очень читаемо и логично, видно опыт. Но порой вот такое вот.

  3. McHummer1 говорит:

    А под какой микрик этот код? stm32?

  4. vam говорит:

    В очень былые времена «микриком» был только микропереключатель ;)

  5. Иван говорит:

    Здравствуйте, как можно с Вами связаться по поводу разработки электроники? Ответьте пожалуйста на почту. Спасибо!

  6. eugene7 говорит:

    А как тут зарегистрироваться?

    • DI HALT говорит:

      Вы уже. А в чем проблема?

      • eugene7 говорит:

        Немного странно. Кнопка «войти» есть, а кнопки «регистрация» нет. Коммент отправился, но войти опять же не могу. Нотификации никакой на почту не пришло. При попытке восстановить пароль пишет, что такого email нет.
        Я чего-то не понимаю?

        • DI HALT говорит:

          Да, в самом деле. Регистрация похоже отвалилась где то по пути :))) И вы не зарегистрированы, по крайней мере по имени среди зарегленых пользователей вас не видно.

          Впрочем, а оно вам тут надо? Комментарии и так остаются. В базе вы есть и новые ваши комментарии даже на модерацию не должны попадать.

  7. eugene7 говорит:

    Вроде комменты отправлять могу, поэтому теперь замечания по теме.
    Не уверен, что эта тема еще актуальна, но не могу не высказаться. :)

    Может я ошибаюсь, и с теми типами данных, с которыми вы работаете, все ОК, но вообще в этих 2-х функциях несколько ошибок. Видно, что писал человек без особого опыта, который еще не успел наступить на эти грабли :)

    1. В ADC_GetValue_ma_x100 надо сначала умножать, а потом делить, иначе ухудшается точность. При этом, конечно, надо проверить диапазоны значений, чтобы при умножении не происходило переполнение. Не указано какого типа pADC_Calibrate_K2 и pADC_Calibrate_K1, но предполагаю, что они также uint16_t. (Вообще, кто именует переменные с маленького p? Обычно это
    признак указателя).
    Пример данных при которых будет грубое округление:
    Integrator = 13, pADC_Calibrate_K2 = 7, pADC_Calibrate_K1 = 3.
    Результат: 13/7=1, 1*3=3.
    Если делать наоборот, то 13*3=39, 39/7=5.
    Математически правильный ответ 5.57, и очевидно, что 5 ближе к правильному ответу чем 3.

    2. В функции PressureSensorGetPressure переменную Temp совершенно бессмысленно делать uint32_t. Если цель этого была чтобы промежуточые вычисления приводились к uint32_t то неявное приведение так не работает. Оба операнда сначала приводится максимальному типу одного из них, вычисляется результат, а потом этот результат приводится к типу переменной, которой этот результат присваевается.
    Например,
    int x = 12;
    int y = 7;
    float z = x / y;
    Результат будет равен 1.
    Поэтому надо либо явно привести один из операндов к uint32_t либо сделать переменную Temp uint16_t.
    Тем более, что фунция PressureSensorGetPressure все равно возвращает uint16_t.

    3. И самое важное. Уважаемый DI HALT поборол TOCTOU, но не до конца. :)
    Проявляться эта проблема должна гораздо реже и не так заметно, но должна. Опять же не видно какого типа MeasuredValues, но предполагаю, что uint16_t, (иначе зачем Integrator uint32_t, можно было ограничиться uint16_t). Так вот на 8-битных AVR доступ к 16-битным переменным НЕ АТОМАРНЫЙ.
    То есть, мы можем из основного потока мы можем начать вычитывать один из элементов массива в регистры. Вычитали младшую часть, и тут приходит прерывание, которое обновляет этот элемент в таблице. Вторая половина числа оказывается не той, которая была изначально.
    Побороть это можно несколькими способами. Самые простые:
    a) Не перезапускать измерение на АЦП, пока не обработаются уже собранные
    данные. При этом придется отказаться от continues conversation mode АЦП,
    но с другой стороны, зачем постоянно получать данные, которые в итоге не
    используются.
    б) Запрещать прерывания в ADC_GetValue_ma_x100 внутри цикла. Ну тут надо
    смотреть, насколько это критично. Тем более можно запрещать не глобальные
    прерывания, а только прерывания от АЦП.
    Возможны другие варианты, тут уж насколько позволяет архитектура проекта и
    фантазия.

    • DI HALT говорит:

      1. Хм, и в самом деле. Впрочем это пофигу, точности хватает с избытком даже так.
      2. Если я правильно понял, то это сделано чтобы не вылезла за размер в ходе вычислений.
      3. Нет, там все атомарно выходит. Я смотрел ,проц 32 разрядный.

      • eugene7 говорит:

        2. Не, в таком виде это не помогает, а даже чуточку вредит :)
        3. Выше в комментах ты писал, что проц ATMega128. А это таки 8-битный AVR
        https://www.microchip.com/wwwproducts/en/ATmega128

        • DI HALT говорит:

          А чем вредит?

          Да, был на авр когда то. Сейчас уже нет.

          • eugene7 говорит:

            Вредит тем, что ошибочно кажется, что вычисления происходят в 32-битном диапазоне, а на самом деле в 16-битном. Еще один нюанс, что компайлер возможно сгенерит лишний код, когда _после_ вычисления будет расширять результат до 32-х бит, а потом все равно обрежет, потому что функция возвращает 16 бит. Но насчет последнего утверждения я не уверен, возможно какие-то компайлеры смогут это соптимизировать и не будут генерить код для расширения переменой. Если проц 32-битный, то это вероятно не важно, потому что там скорее всего компайлер оперирует 32-битными разрядными регистрами, поэтому расширение получается автоматически. Он просто не будет обнулять старшую часть да и все.

Добавить комментарий

Ваш e-mail не будет опубликован. Обязательные поля помечены *

Этот сайт использует Akismet для борьбы со спамом. Узнайте как обрабатываются ваши данные комментариев.