[DiMonstr 0]

...Если так поменять, тогда вообще никакого свитча не понадобится, и размер кода заметно уменьшиться. Такие места есть ещё.

Да зачем я буду обежаться, я и разместил здесь проект, чтобы в свой адрес услышать критику. Что и как я сделал не правильно, почему, как надо и т.д. Я же не профессионал, поэтому и прошу совета.

За предложение forever failure благодарю! Буду переделывать.

 

И ещё много чего интересного и занимательного есть, из-за чего этот код рабочим не является, и, похоже, никогда таковым не был.

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

 

 

Какие ещё есть косяки в коде? Огласите пожалуйста.

 

 

1) if (Value > MinValue & Value < MaxValue) /*<- здесь действительно побитовое И требуется ?*/

2) MeanValue_CH1 = NULL; /* почему, кстати, целочисленной переменной присваивается значание указателя ? */

1) Никак нет, в данном случае производится элементарное сравнение - логическое И значит.

2) В данном случае переменная обнуляется.

Изменено 21 января, 2008 пользователем DiMonstr

[Сергей Борщ 140]

1) Никак нет, в данном случае производится элементарное сравнение - логическое И значит.

Тогда и надо писать if (Value > MinValue && Value < MaxValue). Код будет значительно компактнее и быстрее.

2) В данном случае переменная обнуляется.

Тогда так и надо писать: MeanValue_CH1 = 0. NULL имеет тип указателя, удивительно что компиялтор пропустил такое присваивание (если только в режиме C++, но в нем как раз рекомендуется использовать 0 в том числе и для указателей вместо NULL).

[forever failure 0]

/* if (Value > MinValue & Value < MaxValue) */ В таком случае просмотрите весь проект.

Таких мест я там заметил немало. И ещё, для большеё ясности, что требуется по логике работы, в операторах ветвления (if, for, while, ...) приоритет операций сравнения лучше определять явным образом, т. е. скобками:

if ((Value > MinValue) && (Value < MaxValue)). Или тщательно изучить главу про приоритеты операторов C, дабы не было неожиданностей.

[SapegoAL 0]

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

 

Либо локализуйте ошибку - либо выкиньте часть текста, до которого всё равно дело не доходит и тогда обращайтесь. Простите, это моё мнение.

 

По поводу оптимизации. В варианте со switch компилятор, тем не менее сам найдёт общие куски и объединит их. Я бы для начала отладил бы прогу в целом, а уж потом занялся оптимизацией проекта (если уж сразу не удалось написать оптимально). А то сейчас начать оптимизацию - это добавить новых ошибок.

[Qwertty 0]

NULL имеет тип указателя, удивительно что компиялтор пропустил такое присваивание (если только в режиме C++, но в нем как раз рекомендуется использовать 0 в том числе и для указателей вместо NULL).

Я вот всю жизнь считал что NULL имеет тип unsigned int, такой же как указатель, но это все же не одно и тоже. Так что при присваивании переменная = NULL произойдет обычное преобразование типов, если это нужно, и никаких варнингов не последует.

[defunct 0]

Я вот всю жизнь считал что NULL имеет тип unsigned int, такой же как указатель, но это все же не одно и тоже.

 

typedef void *PVOID;

#define NULL ((PVOID)(0))

 

 

DiMonstr

Нашел у вас в коде такие записи:

BufLog[0] = StartByte;

BufLog[1] = MSB(tics);

BufLog[2] = LSB(tics);

BufLog[3] = seconds;

BufLog[4] = minutes;

BufLog[5] = hours;

BufLog[6] = day;

BufLog[7] = month;

BufLog[8] = MSB(year);

BufLog[9] = LSB(year);

BufLog[10] = StopByte;

 

Но ведь можно просто объявить структуры, где будут храниться ваши записи, например:

typedef struct tagRTC_TIME
{
    U16 Ticks;
    U8   seconds;
    U8   minutes;
    U8   hours;
    U8   day;
    U8   month;
    U16 year;
} TRTC_TIME, *PRTC_TIME;

TRTC_TIME rtcTime;

 

И далее в отчет просто копировать всю запись целиком одной строкой:

 

memcpy( &BufLog[ 1 ], &rtcTime, sizeof( rtcTime ));

 

При отправке этого BufLog'a, или записи в eeprom - подцеплять "старт" и "стоп" таги.

[Qwertty 0]

typedef void *PVOID;

#define NULL ((PVOID)(0))

Да, был неправ. Нашел целых три определения:

#define NULL __null

#define NULL ((void *)0)

#define NULL 0

Второе - С, третье С++, а первое непонятно...

[DiMonstr 0]

Тогда и надо писать if (Value > MinValue && Value < MaxValue). Код будет значительно компактнее и быстрее.

Т.е., как я понимаю, результат при сравнении операторами '&' или '&&' будет одинаков, за исключением того, что оператор '&' делает это побитно, а '&&' побайтно. Верно?

 

Тогда так и надо писать: MeanValue_CH1 = 0. NULL имеет тип указателя, удивительно что компиялтор пропустил такое присваивание (если только в режиме C++, но в нем как раз рекомендуется использовать 0 в том числе и для указателей вместо NULL).

В файле <stdio.h> NULL объявлена как 0:

#ifndef NULL

#define NULL (0)

#endif

 

И в других хидерах переопределений нет. Поэтому так и написал. А то, что NULL имеет тип указателя я и не знал. :05:

Для надежности всё же исправил.

 

Логика такая: при выходе измеряемого напряжения (Value) за допустимые значения (MinValue и MaxValue) необходимо отключать питание системы.

Т.е. в данном случае, если условие соблюдается, то всё в порядке, иначе блокируем работу системы.

В отладчике проверял - работает. На практике проверял - тоже работает.Поэтому так и оставил.

Теперь исправил. :)

 

 

Либо локализуйте ошибку - либо выкиньте часть текста, до которого всё равно дело не доходит и тогда обращайтесь. Простите, это моё мнение.

По поводу оптимизации. В варианте со switch компилятор, тем не менее сам найдёт общие куски и объединит их. Я бы для начала отладил бы прогу в целом, а уж потом занялся оптимизацией проекта (если уж сразу не удалось написать оптимально). А то сейчас начать оптимизацию - это добавить новых ошибок.

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

От свитча я вовсе избавился, действительно через массив оказалось всё просто, компактно и эффективно.

Оптимизация у меня давно выставлена по размеру и я её больше не трогаю с тех пор как компилятор по мере реализации новых фунций начал выдавать такие сообщения:

Building configuration: pwr_control - Release 
Updating build tree... 
main.c 
Linking 
Error[e16]: Segment CODE (size: 0x202e align: 0x1) is too long for segment definition. At least 0xa0 more bytes  
needed. The problem occurred while processing the segment placement command  
"-Z(CODE)CODE=26-1FFF", where at the moment of placement the available memory ranges were  
"CODE:72-1fff" 
   Reserved ranges relevant to this placement: 
   CODE:26-3d           NEAR_F 
   CODE:3e-65           SWITCH 
   CODE:66-71           INITTAB 
   CODE:72-1fff         ?FILL1 
Error[e16]: Segment TINY_ID (size: 0 align: 0) is too long for segment definition. At least 0 more bytes needed.  
The problem occurred while processing the segment placement command  
"-Z(CODE)TINY_ID,NEAR_ID=26-1FFF", where at the moment of placement the available memory ranges  
were "-none-" 
   Reserved ranges relevant to this placement: 
   CODE:26-3d           NEAR_F 
   CODE:3e-65           SWITCH 
   CODE:66-71           INITTAB 
   CODE:72-1fff         ?FILL1 
Error[e16]: Segment NEAR_ID (size: 0xe1 align: 0) is too long for segment definition. At least 0xe1 more bytes  
needed. The problem occurred while processing the segment placement command  
"-Z(CODE)TINY_ID,NEAR_ID=26-1FFF", where at the moment of placement the available memory ranges  
were "-none-" 
   Reserved ranges relevant to this placement: 
   CODE:26-3d           NEAR_F 
   CODE:3e-65           SWITCH 
   CODE:66-71           INITTAB 
   CODE:72-1fff         ?FILL1 
Error[e16]: Segment CHECKSUM (size: 0x2 align: 0) is too long for segment definition. At least 0x2 more bytes  
needed. The problem occurred while processing the segment placement command  
"-Z(CODE)CHECKSUM#1FFF", where at the moment of placement the available memory ranges were  
"-none-" 
   Reserved ranges relevant to this placement: 
   CODE:0-17            INTVEC 
   CODE:18-25           NEAR_ID 
   CODE:26-3d           NEAR_F 
   CODE:3e-65           SWITCH 
   CODE:66-71           INITTAB 
   CODE:72-1fff         ?FILL1 
Error[e18]: Range error,  
Address out of range. Valid range is 0 to 8190 (0x1FFE). 
File: E:\_CRYPTOSOFT\PWR_CONTROL\SRC\util.c, Line: 240  
Source: RCALL   ?PROLOGUE4_L09 
  Where $ = slow_crc16 [0x370] 
            in module "main" (E:\_CRYPTOSOFT\PWR_CONTROL\SRC\Release\Obj\main.r90), 
            offset 0x0 in segment part 159, segment CODE 
  What: ?PROLOGUE4_L09 [0x2000] 
  Allowed range: 0x0 - 0x1FFE 
  Operand: ?PROLOGUE4_L09 [0x2000] 
           in module ?PROLOGUE_L09 (D:\Program Files\IAR Systems\Embedded Workbench 4.0 Evaluation\ 
avr\LIB\CLIB\cl1s-ec_mul.r90), 
           Offset 0x0 in segment part 12, segment CODE 
Error[e18]: Range error,  
Address out of range. Valid range is 0 to 8190 (0x1FFE). 
File: E:\_CRYPTOSOFT\PWR_CONTROL\SRC\util.c, Line: 257  
Source: RJMP    ?EPILOGUE_B4_L09 
  Where $ = slow_crc16 + 0x56  [0x3C6] 
            in module "main" (E:\_CRYPTOSOFT\PWR_CONTROL\SRC\Release\Obj\main.r90), 
            offset 0x56 in segment part 159, segment CODE 
  What: ?EPILOGUE_B4_L09 [0x201A] 
  Allowed range: 0x0 - 0x1FFE 
  Operand: ?EPILOGUE_B4_L09 [0x201a] 
           in module ?EPILOGUE_B_L09 (D:\Program Files\IAR Systems\Embedded Workbench 4.0 Evaluation\ 
avr\LIB\CLIB\cl1s-ec_mul.r90), 
           Offset 0x0 in segment part 12, segment CODE 
Error[e18]: Range error,  
Address out of range. Valid range is 0 to 8190 (0x1FFE). 
File: E:\_CRYPTOSOFT\PWR_CONTROL\SRC\rdc.c, Line: 29  
Source: RCALL   ?PROLOGUE2_L09 
  Where $ = BeginDataWriteToRDC [0x3C8] 
            in module "main" (E:\_CRYPTOSOFT\PWR_CONTROL\SRC\Release\Obj\main.r90), 
            offset 0x0 in segment part 160, segment CODE 
  What: ?PROLOGUE2_L09 [0x2004] 
  Allowed range: 0x0 - 0x1FFE 
  Operand: ?PROLOGUE2_L09 [0x2004] 
           in module ?PROLOGUE_L09 (D:\Program Files\IAR Systems\Embedded Workbench 4.0 Evaluation\ 
avr\LIB\CLIB\cl1s-ec_mul.r90), 
           Offset 0x0 in segment part 14, segment CODE 
.......
ну и так далее
.......
Total number of errors: 33 
Total number of warnings: 0

 

Ну я и воткнул оптимизацию по размеру на максимум. Всё работает, но вот елси разрешить выполнение следующего кода:

#if(CheckMCU)
  sum = slow_crc16(sum,(unsigned char __flash *)0, (unsigned long)&__checksum);
  // call with two 0 bytes for the correct calculation of crc
  sum = slow_crc16(sum,(unsigned char __flash *)&zero,2);                      
...
#endif

то прошивка лобо вообще не запускается либо запускается через раз. А по отдельности эти части кода фунциклируют по честному.

 

 

А я сначала и объявлял переменные через структуру. Только не смог передать содержимое структуры не в массив, не напрямую в eeprom. Я вообще не знал про эту функцию:

memcpy( &BufLog[ 1 ], &rtcTime, sizeof( rtcTime ));

А каким образом можно без неё вообще обойтись и записывать структуру непосредственно в eeprom?

Спасибо за дельный совет.

Изменено 21 января, 2008 пользователем DiMonstr

[SapegoAL 0]

Т.е., как я понимаю, результат при сравнении операторами '&' или '&&' будет одинаков, за исключением того, что оператор '&' делает это побитно, а '&&' побайтно. Верно?

Нет не верно.

выражение внутри скобок if (Value > MinValue && Value < MaxValue) означает что если Value > MinValue И Value < MaxValue, то ... Если вы вместо && поставите &, то это будет означать, что сначала выполнятся два сравнения потом выполнится логическое & над результатами а потом результат будет сравнён с "0". Поскольку значение "true" определена как "не 0", то в общем случае результаты могут не совпадать.

 

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

[aesok 0]

В функциях EEPROM_* - не должны разрешаться прерывания на выходе. Они должны востанавливать состояние состояние флага "I" таким каким оно было при входе в функцию.

 

И вообще используйте библиотечные функции для доступа к EEPROM. В них небудет детских ошибок.

 

И перед тем как сново захотите использовать их в прерываниях прочитайте про понятие "non-reenterable".

 

Анатолий.

Изменено 21 января, 2008 пользователем aesok

[defunct 0]

А я сначала и объявлял переменные через структуру. Только не смог передать содержимое структуры не в массив, не напрямую в eeprom. Я вообще не знал про эту функцию:

...

А каким образом можно без неё вообще обойтись и записывать структуру непосредственно в eeprom?

Можно написать свою функцию для записи блоков в eeprom, например такую:

 

void store_to_eeprom( int dst_addr, void *pData, int size)
{
      unsigned char *p = (unsigned char *)pData;
      while( size--)
           EEPROM_write_byte( dst_addr++, *p++);
}

 

и передавать структуры посредством указателей:

 

store_to_eeprom( RTC_OFFSET, &rtcTime, sizeof( rtcTime ));

В этой функции Вы можете добалять какую-то служебную информацию, например CRC для записываемого блока или таги для поиска как у Вас было. Так можно сократить количество повторяющегося кода.

 

Если пользуетесь компилятором от IAR, то перед структурой можно поставить ключевое слово __eeprom и она будет располагаться в eeprom'е сразу, и пользоваться ее полями можно точно так же как если бы она лежала в RAM. Но не следует этим злоупотреблять, т.к. надо помнить, что ресурс eeprom ограничен.

 

результат при сравнении операторами '&' или '&&' будет одинаков

Нет не верно.

выражение внутри скобок if (Value > MinValue && Value < MaxValue) означает что если Value > MinValue И Value < MaxValue, то ... Если вы вместо && поставите &, то это будет означать, что сначала выполнятся два сравнения потом выполнится логическое & над результатами а потом результат будет сравнён с "0". Поскольку значение "true" определена как "не 0", то в общем случае результаты могут не совпадать.

Саша, в данном конкретном случае & даст такой же результат как и &&, хотя и более долгим путем :)

Но в общем случае, конечно надо быть внимательным.

Иногда помогает помнить, что в теле if() "==" - это сравнение, а "=" - катастрофа :)

аналогично для & и | ;>

[Freeze 0]

Как я и предполагал с самого начала, вашей программе не хватает оперативной памяти... ерроры это только подтверждают... посмотрите модель памяти (tiny и small) и поставьте small... узнайте диапазон значений всех ваших переменных и задайте им соответственный тип... постарайтесь как можно меньше использовать глобальных переменных... в общем - оптимизируйте...

[SapegoAL 0]

Саша, в данном конкретном случае & даст такой же результат как и &&, хотя и более долгим путем :)

Но в общем случае, конечно надо быть внимательным.

Иногда помогает помнить, что в теле if() "==" - это сравнение, а "=" - катастрофа :)

аналогично для & и | ;>

 

В данном случае я отвечал не на вопрос "будет ли такой же результат", а отвечал на вопрос "как я понимаю". И подтверждаю, что человек неправильно понимает. Компилятор IAR результат логической операции принимает как 0 или 1, но это не гарантируется стандартом о чём я и высказался. Согласно стандарту - 0 и отличное от нуля значение. А это отнюдь не 1. Иными словами если в одном выражении будет к примеру 1 а в другом 2, то согласно стандарту всё будет корректно но логическое & над этими операндами даст 0, а это не верный результат. Конечно это чисто теоретический случай, но понимание работы должно быть полным. Программист не должен полагаться на конкретную реализацию компилятора, а должен ориентироваться на стандарт. Тем более, что эта мелочь действительно приведёт к настоящей ошибке в другом месте. То есть человек должен понимать разницу между побитовой и логической операцией.

 

Сравните например:

79 if(~Flag.Sek)show(1,j);

\ 00000046 01A2 MOVW R21:R20, R5:R4

\ 00000048 01B3 MOVW R23:R22, R7:R6

\ 0000004A E001 LDI R16, 1

\ 0000004C .... RCALL show

 

и

79 if(!Flag.Sek)show(1,j);

\ 00000046 .... LDI R30, Flag

\ 00000048 E0F0 LDI R31, 0

\ 0000004A 8100 LD R16, Z

\ 0000004C FD00 SBRC R16, 0

\ 0000004E C004 RJMP ??main_0

\ 00000050 01A2 MOVW R21:R20, R5:R4

\ 00000052 01B3 MOVW R23:R22, R7:R6

\ 00000054 E001 LDI R16, 1

\ 00000056 .... RCALL show

[Сергей Борщ 140]

Компилятор IAR результат логической операции принимает как 0 или 1, но это не гарантируется стандартом о чём я и высказался. Согласно стандарту - 0 и отличное от нуля значение. А это отнюдь не 1.

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

6.5.13 Logical AND operator

3 The && operator shall yield 1 if both of its operands compare unequal to 0; otherwise, it yields 0. The result has type int.

4 Unlike the bitwise binary & operator, the && operator guarantees left-to-right evaluation; there is a sequence point after the evaluation of the first operand. If the first operand compares equal to 0, the second operand is not evaluated.

Т.е. "ноль" и "не ноль" отностится не к результату, а к операндам.

6.5.13 Logical AND operator

3 The usual arithmetic conversions are performed on the operands.

4 The result of the binary & operator is the bitwise AND of the operands (that is, each bit in the result is set if and only if each of the corresponding bits in the converted operands is set).

Поэтому первое отличие: для A=1 и B=2 A&B=0, A&&B=1.

Второе - для && операнды вычисляются слева направо и если правый операнд равен нулю, то уже сразу известно, что и результат будет равен нулю, поэтому второй операнд вычисляться не будет.

[defunct 0]

Ну это не совсем так и компилятор точно следует стандарту:Т.е. "ноль" и "не ноль" отностится не к результату, а к операндам.

Как я понял - речь шла о том гарантируется ли стандартом то, что результатом любого сравнения вида ">" ">=" и т.п. всегда будет только одно из двух "1" или "0", либо такие сравнения гарантируют только "0" и "!0".

Вы привели цитаты только для & и &&, что не раскрывает вопроса ;>

Хотя что-то мне подсказывает что цитаты для && достаточно.

 

Я всегда считал, что рез-татом любого сравнения будет гарантировано "1" или "0", всвязи с чем часто пользуюсь такими конструкциями:

 

x = (y & 1 << somebit ) != 0;

 

для "засовывания" интересующего меня бита в бит0.