Вопросы по С/С++ (СИ)

http://www.tutorialspoint.com/online_c_formatter.htm
Ваш последний код :

void contr_eep (void)
{   
for(comp=0;comp<66;comp++){
 if(mass[comp]!=eep_mass[comp])result=0;}
 if(result==1){delayt=0;while(delayt<200){lcd_gotoxy(0,0);lcd_putsf("  TEST EEPROM         OK!       ");}w=0;}else
{lcd_gotoxy(0,0);lcd_putsf("  TEST EEPROM       FAILURE!    ");if(eeppp&0b0011111110010000){PORTB.5=0;}else{PORTB.5=1;}if(PINC.3==0)
{PORTB.5=0;dreb_0=6;w=0;}}  //пищит каждые примерно 2 минуты
}

в два клика превращается вот в это чудо :

Код: Выделить всё

void contr_eep (void)
{
    for(comp=0; comp<66; comp++) {
        if(mass[comp]!=eep_mass[comp])result=0;
    }
    if(result==1) {
        delayt=0;
        while(delayt<200) {
            lcd_gotoxy(0,0);
            lcd_putsf("  TEST EEPROM         OK!       ");
        }
        w=0;
    }
    else
    {   lcd_gotoxy(0,0);
        lcd_putsf("  TEST EEPROM       FAILURE!    ");
        if(eeppp&0b0011111110010000) {
            PORTB.5=0;
        }
        else {
            PORTB.5=1;
        }
        if(PINC.3==0)
        {
            PORTB.5=0;    //пищит каждые примерно 2 минуты
            dreb_0=6;
            w=0;
        }
    }
}

Приятнее смотреть, не правда ли ?

Прогоняйте свой код, хотябы, через подобные сервисы, перед выкладыванием. И ни у кого не возникнет никаких вопросов и лишних слов

Реклама

ra9ust

Точно работает! Не знал про такие сервисы, спасибо за подсказку!

Реклама

Что касается кода. Я, например, с трудом представляю, что в этом коде делается.
Например, вот тут :

Код: Выделить всё

        delayt=0;
        while(delayt<200) {
            lcd_gotoxy(0,0);
            lcd_putsf("  TEST EEPROM         OK!       ");
        }

я вижу бесконечный цикл.

А этот код вообще бессмыселен :

Код: Выделить всё

    for(comp=0; comp<66; comp++) {
        if(mass[comp]!=eep_mass[comp])result=0;
    }
    if(result==1) {
        ....

ra9ust

Ну как же так, в первом цикл не бесконечен, а длится 2 сек для отображения надписи епром ок и после этого возврат в главное меню(w=0).
Во втором сравниваются 65 переменных в озу с их значениями в епром, если успешно то пишется та самая надпись Ок на две сек, если хоть одно отличие то другая надпись и звуковое оповещение, всё работает, проверено.Это замена той вчерашней кошмарной записи где каждая переменная проверяется через if.

Реклама

ra9ust писал(а):цикл не бесконечен, а длится 2 сек

Не вижу там 2-ух секунд. Вижу, что цикл будет крутиться бесконечно, ибо переменная всегда будет равна нулую.
Он может у Вас где-то и изменяться (например, в прерывании), но нам этого не видно. Отсюда и тяжело анализировать код, не имея о нём представления.

ra9ust писал(а):Во втором сравниваются 65 переменных в озу с их значениями в епром, если успешно то ..........

А представьте, что на начале цикла, переменная result будет равна нулю. Что произойдёт ?

Реклама

ra9ust

delayt постоянно инкрементируется в 10мс прерывании.

result Не будет равна нулю, в самом начале , после инициализации портов и устройств, перед вечным циклом ей присваивается 1.

Добавлено after 2 minutes 27 seconds:
Хотя можно и в инверсии сделать, код сократится на одно присваивание.

Реклама

ARV · Сообщение **ARV** » Вт окт 31, 2017 17:40:08

ra9ust писал(а):result Не будет равна нулю, в самом начале , после инициализации портов и устройств, перед вечным циклом ей присваивается 1

а как же это:

Аlex писал(а):if(mass[comp]!=eep_mass[comp])result=0;

если данные не совпадут?

ra9ust писал(а):после инициализации портов и устройств, перед вечным циклом ей присваивается 1

1. Место её инициализации - прямо перед самым циклом, а не "где-то там...". Мало того, она должна быть локальной, ибо используется только в одном месте.
2. Функция проверки еепром должна содержать только код ля её проверки. Никаких дисплеев, пищалок, и т.д... там не должно быть.
3. ... Уххх, много чего можно ещё писать, руки устанут

ARV · Сообщение **ARV** » Вт окт 31, 2017 17:41:41

и в течение 2 секунд каждые 10 мс выводить одно и то же на дисплей - это крутой подход, логичный вполне. не тупая же задержка...

ra9ust

Ну так и будет вывод соответствующей надписи else
{lcd_gotoxy(0,0);lcd_putsf(" TEST EEPROM FAILURE! ");

А что за слово такое "Failure" ?

ra9ust

Почему нежелательно инициализация где-то там и в чем минус того что она глобальная?
Почему никаких дисплеев и пищалок?

Добавлено after 2 minutes 24 seconds:
[uquote="Аlex",url="/forum/viewtopic.php?p=3221168#p3221168"]А что за слово такое "Failure" ?

[/uquote]
К исполняемому коду это не имеет никакого отношения!

Добавлено after 1 minute 29 seconds:
[uquote="ARV",url="/forum/viewtopic.php?p=3221163#p3221163"]и в течение 2 секунд каждые 10 мс выводить одно и то же на дисплей - это крутой подход, логичный вполне. не тупая же задержка...[/uquote]
Напишите как по другому...

viiv

[uquote="ra9ust",url="/forum/viewtopic.php?p=3220997#p3220997"]Ну так это всё у меня и сделано, естественно так как мне удобно.[/uquote]

Где же так?

Код: Выделить всё

void contr_eep (void)
{   
for(comp=0;comp<66;comp++){
 if(mass[comp]!=eep_mass[comp])result=0;}
 if(result==1){delayt=0;while(delayt<200){lcd_gotoxy(0,0);lcd_putsf("  TEST EEPROM         OK!       ");}w=0;}else
{lcd_gotoxy(0,0);lcd_putsf("  TEST EEPROM       FAILURE!    ");if(eeppp&0b0011111110010000){PORTB.5=0;}else{PORTB.5=1;}if(PINC.3==0)
{PORTB.5=0;dreb_0=6;w=0;}}  //ïèùèò êàæäûå ïðèìåðíî 2 ìèíóòû
}

66 размер массива? Вот Вы решите добавить еще параметр (или убрать). Придется весь код просматривать и править - не осталось ли где 66. Причем, например, при выходе за границу массива, Вы можете не увидеть сразу проблему. Сейчас надо посмотреть объявления mass и eep_mass, чтобы убедиться, что нет ошибки.
comp почему не локальная? Чтобы убедиться, что ошибки нет - надо просмотреть весь текст.
result Также глобальная. Чтобы убедиться, что ошибки нет - надо просмотреть весь текст.
delayt Также глобальная. Чтобы убедиться, что ошибки нет - надо просмотреть весь текст. А ошибка скорей всего есть, так как данная переменная много где используется (судя по текстам, которые Вы давали ранее), в том числе и в обработчике прерываний.
w, опять же глобальная. Назначение непонятное, к тому же поиск затруднен (переменная очень короткая). Много где используется, в том числе и в обработчике прерываний. Скорей всего есть ошибка (и даже не одна).
eeppp&0b0011111110010000 - назначение eeppp и константы непонятно, модифируется в обработчике, используется вне обработчика: как volatile не объявлена, атомарность доступа "побоку". Лень смотреть, но скорей всего есть ошибка.
PORTB.5 PINC.3 непонятно как используются, но используется много где. Если PORTB.5 - это включение alarm-а, то почему не сделать функцию

Код: Выделить всё

void set_alarm (bool_t on)
{
    PORTB.5 = on ? 1 : 0;
}

Так ведь понятнее.

Не много ли для 8-ми строк кода? После внесения правок надо бы проверить все заново.

[uquote="ra9ust",url="/forum/viewtopic.php?p=3221121#p3221121"]А мне главное чтобы работало[/uquote]
В том то и дело, что работает "криво". Я за пару минут нашел ошибку, с которой Вы даже согласились. Поверьте ошибок у Вас куча. Не удивлюсь, если иногда устройство "глючит" и подвисает. Структурированный и понятный код - это не самоцель, это способ значительно уменьшить количество ошибок в ПО и время для исправления ошибок, расширения функциональности.

Но, если Вам удобнее, то Ваше дело - пишите, как хотите.

ra9ust

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

ra9ust писал(а):Почему нежелательно инициализация где-то там и в чем минус того что она глобальная?

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

ra9ust писал(а):Почему никаких дисплеев и пищалок?

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

PS: Ладно, Вы вместо критики, пытаетесь отбрыкиваться и доказать, что Ваш стиль не хуже других. О чём дальше можно вести беседу ?

arkhnchul

[uquote="ra9ust",url="/forum/viewtopic.php?p=3221170#p3221170"]Почему нежелательно инициализация где-то там[/uquote]
читаемость, логическая связность кода.
[uquote="ra9ust",url="/forum/viewtopic.php?p=3221170#p3221170"]и в чем минус того что она глобальная?[/uquote]
по семантике она локальная. Плюс не факт, что вы очень общее название result не захотите использовать где-то еще. Плюс компилятор может оптимизировать использование локальной переменной, заменив обращение к ней операциями целиком в регистрах.
[uquote="ra9ust",url="/forum/viewtopic.php?p=3221170#p3221170"]Почему никаких дисплеев и пищалок?[/uquote]
логическое разделение и структурирование кода. Функция проверки чего-то должна проверять это что-то; реагировать на результат проверки - дело того, кто функцию вызвал.
[uquote="ra9ust",url="/forum/viewtopic.php?p=3221170#p3221170"]Напишите как по другому...[/uquote]

Код: Выделить всё

lcd_gotoxy(0,0);
lcd_putsf("  TEST EEPROM         OK!       ");
delayt=0;
while(delayt<200); // do nothing, wait for delayt increment

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

Код: Выделить всё

delayt=200;
while(delayt>0); // do nothing, wait for delayt decrement

viiv

[uquote="ra9ust",url="/forum/viewtopic.php?p=3221192#p3221192"]Не понятно что вы подразумеваете под ошибкой? Если сбой в работе или нарушение алгоритма, то всё работает без единого сбоя,без глюков и практически не выключаясь уже полгода,всё как и задумывал работает, ну а если под ошибкой подразумевается просто
загроможденность кода лишними действиями,то возможно соглашусь, потому и спрашивал про упрощение.[/uquote]

Я же Вам показал ошибку. И Вы даже согласились. Или мне еще проанализировать и сказать, в каких случаях Вы ее увидите и что увидите?
И показал возможные проблемы, где еще могут быть проблемы. А зачем? Вас же все устраивает.

ra9ust

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

Добавлено after 2 minutes 19 seconds:
[uquote="viiv",url="/forum/viewtopic.php?p=3221200#p3221200"][uquote="ra9ust",url="/forum/viewtopic.php?p=3221192#p3221192"]Не понятно что вы подразумеваете под ошибкой? Если сбой в работе или нарушение алгоритма, то всё работает без единого сбоя,без глюков и практически не выключаясь уже полгода,всё как и задумывал работает, ну а если под ошибкой подразумевается просто
загроможденность кода лишними действиями,то возможно соглашусь, потому и спрашивал про упрощение.[/uquote]

Я же Вам показал ошибку. И Вы даже согласились. Или мне еще проанализировать и сказать, в каких случаях Вы ее увидите и что увидите?
И показал возможные проблемы, где еще могут быть проблемы. А зачем? Вас же все устраивает.[/uquote]
Я считаю ошибкой то что в какой-то момент происходит сбой или зависание или еще что, никаких сбоев не было, ни изначально еще когда в протеусе проверял, ни в процессе работы уже в железе, режимы все работают уж какие еще нужны ситуации я не знаю, не так уж там и много то функций.

Добавлено after 2 minutes 8 seconds:
Уменьшение счетчика учтем, пожалуй это наглядней чем увеличивать.

viiv

Вы тролите?

eep_month и eep_year присваивается ДО корректировки month и year (т.е. возможно неправильными значениями) Именно ошибка в Вашем понимании.

Поставте 23:59:30 31 декабря. 2017

После завершения минуты у Вас в EEPROM будет (32 декабря 2017 года, вместо 1 января 2018):
eep_day=32
eep_month=12
eep_year=2017

Это не ошибка? Или Вы так задумывали? Ну тады как-то работает и ладно.

arkhnchul

[uquote="ra9ust",url="/forum/viewtopic.php?p=3221201#p3221201"]Уменьшение счетчика учтем, пожалуй это наглядней чем увеличивать.[/uquote]
еще нагляднее - выделить в функцию или макрос

Код: Выделить всё

void delay_ms(uint16_t mseconds){ // 10ms precision
    delayt=mseconds/10;
    while(delayt>0); // do nothing, wait for delayt decrement
}

Код: Выделить всё

#define DELAY_MS(mseconds) ({ \
    delayt=(mseconds)/10; \
    while(delayt>0); \
})