Форум разработчиков электроники ELECTRONIX.ru > Как "красиво" написать этот кусочек?

Pyku_He_oTTyda

Mar 22 2007, 21:44

Вот такой кусочек кода, как написать его покрасивее?
Заранее благодарен за внимание!

Код

void read_key(void)
{
if(PINA==0xFE)
{
up_press=1;
}
else
{
up_press=0;
};

if(PINA==0xFD)
{
down_press=1;
}
else
{
down_press=0;
};

if(PINA==0xFB)
{
left_press=1;
}
else
{
left_press=0;
};

if(PINA==0xF7)
{
right_press=1;
}
else
{
right_press=0;
};

if(PINA==0xEF)
{
mode_press=1;
}
else
{
mode_press=0;
};

if(PINA==0xDF)
{
set_press=1;
}
else
{
set_press=0;
};
}

_sR_

Mar 22 2007, 22:06

Цитата(Pyku_He_oTTyda @ Mar 22 2007, 21:44)

Вот такой кусочек кода, как написать его покрасивее?
Заранее благодарен за внимание!

не совсем понятен смысл key_press
потому как если PINA!=0xDF, то key_press = 0, вне зависимости от предыдущих кнопок.
красота понятие относительное...
как вариант заменить if/else на условный оператор, что немного сократит текст, но не факт, что станет красивее.

Код

up_press = PINA==0xFE ? 1 : 0;

в делнейшем лучше сделать переменные состояния типом bool, тогда возможно будет написать

Код

up_press = PINA==0xFE;

как то так...

singlskv

Mar 22 2007, 22:08

Цитата(Pyku_He_oTTyda @ Mar 22 2007, 21:44)

Вот такой кусочек кода, как написать его покрасивее?
Заранее благодарен за внимание!

Код

............

А у Вас несколько кнопок не могут одновременно быть нажаты ?
Если могут, вот так
PINA==0xFE
писать нельзя.

И что значит покрасивее ?
Что экономим ? flash или RAM ?
нужно быстрее или короче код ?
и т.д. ....
и какой компилятор ?

Сергей Борщ

Mar 22 2007, 22:09

Цитата(Pyku_He_oTTyda @ Mar 22 2007, 20:44)

Вот такой кусочек кода, как написать его покрасивее?

Смотря что подразумевать под "красивее". можно так:

Код

#define    KEY_UP        (1<<0)
#define    KEY_DOWN    (1<<1)
#define    KEY_LEFT    (1<<2)
#define    KEY_RIGHT    (1<<3)
#define    KEY_MODE    (1<<4)
#define    KEY_SET        (1<<5)

void read_key()
{
   uint8_t Port_Val = PINA;
   if(!(Port_Val & KEY_UP))
       up_press = 1;
   else
       up_press = 0;

  .......
   if( !(Port_Val & (KEY_UP | KEY_DOWN | KEY_LEFT | KEY_RIGHT | KEY_SET | KEY_MODE)))
       key_press = 1;
   else
       key_press = 0;
}

А можно так:

Код

#define    KEY_UP        (1<<0)
#define    KEY_DOWN    (1<<1)
#define    KEY_LEFT    (1<<2)
#define    KEY_RIGHT    (1<<3)
#define    KEY_MODE    (1<<4)
#define    KEY_SET        (1<<5)

enum keypressed_t {RELEASED, UP, DOWN, LEFT, RIGHT, MODE, SET};
keypressed_t Key;
void read_key()
{
   uint8_t Port_Val = PINA;
   if(!(Port_Val & KEY_UP))
       Key = UP;
   else
       if(!(Port_Val & KEY_DOWN))
           Key = DOWN;
      .......

                 else
                    Key = RELEASED;
}

void check_key()
{
    switch(Key)
    {
      case UP:
           .....
           break;
      case DOWN:
           .....
           break;
      case RELEASED:
      default:
           .....
           break;
    }
}

Pyku_He_oTTyda

Mar 22 2007, 22:26

Спасибо!
Несколько кнопок не могут быть нажаты, это считается ошибкой.

З.Ы. Прошу прощения, с использованием копи-пасте пропустил ошибку, key_press в моем коде не работает, счас поправлю

Сергей Борщ

Mar 22 2007, 22:37

Цитата(Pyku_He_oTTyda @ Mar 22 2007, 21:26)

Спасибо!
Несколько кнопок не могут быть нажаты, это считается ошибкой.

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

Код

enum keypressed_t {RELEASED, UP, DOWN, LEFT, RIGHT, MODE, SET, MAX_KEY};
void read_key()
{
   uint8_t Port_Val = ~PINA;
   uint8_t Mask = 1 << (MAX_KEY - 1);
   uint8_t Key_tmp = MAX_KEY - 1;
   do
   {
     if(Port_Val & Mask)
       break;
     Mask >>= 1;
     Key_tmp--;
   }
   while(Mask);
   Key = Key_tmp;
}

singlskv

Mar 22 2007, 22:40

А значения up_press, down_press , .....
нельзя объединить в массив ?
по моему это сильно упростило бы код

Цитата(Pyku_He_oTTyda @ Mar 22 2007, 22:26)

Несколько кнопок не могут быть нажаты, это считается ошибкой.

то есть если несколько нажато, то считаем что не нажато ни одной ?

Pyku_He_oTTyda

Mar 22 2007, 22:44

Цитата

то есть если несколько нажато, то считаем что не нажато ни одной ?

Совершенно верно!

singlskv

Mar 22 2007, 22:48

Цитата(Сергей Борщ @ Mar 22 2007, 22:37)

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

Код

if(Port_Val & Mask)
break;
}

так мы ловим первую по списку нажатую
чтобы проверить что нажата только 1 нужно заменить break; на например tmp++;
и в конце цикла проверять tmp<2

Сергей Борщ

Mar 22 2007, 22:57

Цитата(singlskv @ Mar 22 2007, 21:48)

так мы ловим первую по списку нажатую
чтобы проверить что нажата только 1 нужно заменить break; на например tmp++;
и в конце цикла проверять tmp<2

Да, про то, что две нажатые считаются как "ни одной" на момент написания не было. Тогда можно или считать, или так:

Код

void read_key()
{
   uint8_t Port_Val = ~PINA;
   Port_Val &= (1<<MAX_KEYS) - 1;    // mask unused bits
   if(!(Port_Val & (Port_Val - 1)))  // if only one key pressed
   {
      uint8_t Mask = 1 << (MAX_KEY - 1);
      uint8_t Key_tmp = MAX_KEY - 1;
      do
      {
        if(Port_Val & Mask)
           break;
        Mask >>= 1;
        Key_tmp--;
      }
      while(Mask);
      Key = Key_tmp;
   }
   else
      Key = RELEASED;
}

singlskv

Mar 22 2007, 23:41

Цитата(Сергей Борщ @ Mar 22 2007, 22:57)

Да, про то, что две нажатые считаются как "ни одной" на момент написания не было. Тогда можно или считать, или так:

Код

void read_key()
{
   uint8_t Port_Val = ~PINA;
   Port_Val &= (1<<MAX_KEYS) - 1;    // mask unused bits
   if(!(Port_Val & (Port_Val - 1)))  // if only one key pressed
   {
                 uint8_t Mask = 1 << (MAX_KEY - 1);
      uint8_t Key_tmp = MAX_KEY - 1;
      do
      {
        if(Port_Val & Mask)
           break;
        Mask >>= 1;
        Key_tmp--;
      }
      while(Mask);
      Key = Key_tmp;
   }
   else
      Key = RELEASED;
}

Да, наверное так будет оптимально
только вот здесь: uint8_t Mask = 1 << (MAX_KEY - 1);
случайно не -2 ?

SasaVitebsk

Mar 23 2007, 00:30

А если так???

Код

#define                CNT_KEY        6                        // Число кнопок
__no_init    struct
{
uint8_t            Up            : 1,                    // Вверх
                            Down        : 1,                    // Вниз
                            Left        : 1,                    // Влево
                            Right        : 1,                    // Вправо
                            Mode        : 1,                    // Мода
                            Set            : 1;                    // Установить
} Key;

void read_key()
{
uint8_t            i,j,k;

Key = ~PINA;
i = ~Key;
k=0;
for(j=0;j<CNT_KEY;j++) {k += i & 1; i >>= 1};
if(k>1) Key = 0;
}

// А анализировать так

if(Key.Up)
{
....
}

Сергей Борщ

Mar 23 2007, 14:34

Цитата(SasaVitebsk @ Mar 22 2007, 23:30)

А если так???

Код

Key = ~PINA;
i = ~Key;

Присваивание целого структуре и наоборот недопустимо. Тогда уж

Код

union
{
    struct
   {
       uint8_t            Up            : 1,                    // Вверх
       uint8_t            Down        : 1,                    // Вниз
       ..........
    },
    uint8_t raw;
} Key;

Key.raw = ~PINA;
i = ~Key.raw;

SasaVitebsk

Mar 23 2007, 20:57

Ну ты у нас с первого взгляда ....

Я бы ч/з указатели присваивание сделал.

А то как-то вся красота куда-то деётся.

_sR_

Mar 23 2007, 21:19

Цитата(Сергей Борщ @ Mar 23 2007, 14:34)

Присваивание целого структуре и наоборот недопустимо. Тогда уж

Код

union...

поддерживаю
кроме того, не помню, чтобы стандарт гарантировал прямое соответствие битов значения целого и битов
поля.
хотя признаю, что отклонения не встречал, но ведь вопрос был в красоте

smk

Mar 23 2007, 22:30

switch () {
case :
break;

default:
};

Имеет смысл рассмотреть такую конструкцию.

kamedi_clab

Mar 23 2007, 23:44

Цитата(smk @ Mar 23 2007, 22:30)

switch () {
case :
break;

default:
};

Имеет смысл рассмотреть такую конструкцию.

Предлагали там:
http://forum.ixbt.com/topic.cgi?id=48:5298-12

Говорит не подходит.

Pyku_He_oTTyda

Mar 24 2007, 09:58

Спасибо всем

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

kamedi_clab

Mar 24 2007, 10:11

Надеюсь покажешь как кусочек реализован ?

SasaVitebsk

Mar 24 2007, 15:54

Цитата(_sR_ @ Mar 23 2007, 22:19)

поддерживаю
кроме того, не помню, чтобы стандарт гарантировал прямое соответствие битов значения целого и битов
поля.
хотя признаю, что отклонения не встречал, но ведь вопрос был в красоте

А помоему это красивое решение (хотя понятие субъективное). И что ещё важнее - точно отражает суть. То есть у вас признаки нажатия. Очень читабелен текст.

Код

if(Key.Up)
{
MotorVert(On);
}

Что тут написано вспомнишь ч/з год.

Pyku_He_oTTyda

Mar 24 2007, 21:21

Покажу

proba

Mar 25 2007, 10:46

Цитата(Pyku_He_oTTyda @ Mar 22 2007, 21:44)

Вот такой кусочек кода, как написать его покрасивее?
Заранее благодарен за внимание!

Код

void read_key(void)
{
if(PINA==0xFE)
{
up_press=1;
}
else
{
up_press=0;
};

if - else можно заменить нa <a>?<b> :<c>
Ваш пример будет в длинном виде
PINA==0xFE ? up_press = 1 : up_press = 0;
а в коротком:
up_press = PINA==0xFE ? 1 : 0 ;

_sR_

Mar 25 2007, 19:54

Цитата(SasaVitebsk @ Mar 24 2007, 15:54)

А помоему это красивое решение...

Никто не сказал, что Ваше решение не красиво.
Более того, оно мне тоже нравится и в одном из проектов сделал точно так как Вы. Человек, который в последствии поддерживал этот код, понял все сразу.
Сергей Борщ просто показал как это записать "по циркулю"

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