Skip to content

Conversation

@nikitoz236
Copy link
Contributor

No description provided.


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

Плохой пример:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Файлик называется embedded_c.en.md, а контент на русском.

embedded_c.en.md Outdated

Например вместо:
```C
void input_handler(unsigned input) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

В общем, имхо в самом switch-case нет ничего плохого. И он много где используется у нас. Надо подобрать более подходящий пример, где его лучше не использовать

embedded_c.en.md Outdated

Лушче написать так:
```C
#define INPUTS_NUMBER 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В принципе можно и без этого обойтись, а использовать ARRAY_SIZE

embedded_c.en.md Outdated

# Переиспользование
Нужно использовать код повторно. Не нужно писать код если он уже написан. Если чтото существующее подходит не нужно делать чтото новое.
Чтобы не оставлять ошибки в дубликатах, экономить силы программиста и прочее. Читать подробнее в [Википедии](https://ru.wikipedia.org/wiki/%D0%9F%D0%BE%D0%B2%D1%82%D0%BE%D1%80%D0%BD%D0%BE%D0%B5_%D0%B8%D1%81%D0%BF%D0%BE%D0%BB%D1%8C%D0%B7%D0%BE%D0%B2%D0%B0%D0%BD%D0%B8%D0%B5_%D0%BA%D0%BE%D0%B4%D0%B0).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Может юникодную ссылку вставить?

embedded_c.en.md Outdated


# Поллинг
Не должно быть периодически вызываемых функций с проверкой флагов. Поллинг это плохо. Тратится процессорное время и память для флагов которые проверяются. Также не понятно кто этот флаг ставит, кто его проверяет.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну тоже не согласен. Вот в WBIO поллинг. Написано категорично, переделываем? А отказаться от поллинга не всегда возможно.
Тратится процессорное время и память - тоже так себе аргумент, зачем они нужны, если их не тратить?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

У любого кода есть причина выполнения, вот в обработке причины и нужно вызывать выполнение кода, или запланировать его выполнение.
Используйте инструменты организации кода - корутины и таймменеджер. В прерываниях планируйте задачу с обработкой в бэкграунде. Не используйте флаги.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну сам таймменеджер внутри флаги поллит же

embedded_c.en.md Outdated

# Не пишите лишний код, не выполняйте не нужные действия в коде
Пример: если файл с настройками не найден, прошивка использует дефолтные настройки. Не нужно сохранять файл с дефолтными настройками. Файл нужно записывать только если пользователь чтото поменял.
Чтение файла с дефолтными настройками ничем не отличается от отсутствия файла. По этому можно избавиться от лишней операции записи, расхода места иизноса флеш памяти, кода который эту запись делает.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@nikitoz236 nikitoz236 requested a review from pgasheev September 9, 2025 09:50
Copy link
Contributor

@pgasheev pgasheev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Плюс попросить copilot исправить орфографию и пунктуацию


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

Лушче написать так:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Лушче написать так:
Лучше написать так:

...

gpio_init(INPUT_1);
gpio_init(INPUT_2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А зачем тут отступы? Думаю не нужны

Comment on lines +442 to +446
#define MODBUS_USART_HW { \
.usart = USART1, \
.irqn = USART1_IRQn, \
.dma_tx_ch = DMA1_Channel2 \
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define MODBUS_USART_HW { \
.usart = USART1, \
.irqn = USART1_IRQn, \
.dma_tx_ch = DMA1_Channel2 \
}
#define MODBUS_USART_HW \
{ \
.usart = USART1, \
.irqn = USART1_IRQn, \
.dma_tx_ch = DMA1_Channel2 \
}

Вроде получше выглядит

Пример: если файл с настройками не найден, прошивка использует дефолтные настройки. Не нужно сохранять файл с дефолтными настройками. Файл нужно записывать только если пользователь чтото поменял.
Чтение файла с дефолтными настройками ничем не отличается от отсутствия файла. Поэтому, можно избавиться от лишней операции записи, расхода места и износа флеш памяти, кода который эту запись делает.

# Не нужно делать в run time все что можно сделать в compile time
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это точно всё заголовки первого уровня? Думаю надо подальше задвинуть

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants