Skip to content

Feature/recomendations from google doc#84

Open
nikitoz236 wants to merge 3 commits into
masterfrom
feature/recomendations_from_google_doc
Open

Feature/recomendations from google doc#84
nikitoz236 wants to merge 3 commits into
masterfrom
feature/recomendations_from_google_doc

Conversation

@nikitoz236

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread embedded_c.en.md

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

Плохой пример:

Copy link
Copy Markdown
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, а контент на русском.

Comment thread embedded_c.en.md Outdated

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

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

Comment thread embedded_c.en.md Outdated

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread 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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread embedded_c.en.md Outdated


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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

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

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread embedded_c.en.md Outdated

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

Copy link
Copy Markdown
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

@pgasheev pgasheev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread embedded_c.en.md

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread embedded_c.en.md
...

gpio_init(INPUT_1);
gpio_init(INPUT_2);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread embedded_c.en.md
Comment on lines +442 to +446
#define MODBUS_USART_HW { \
.usart = USART1, \
.irqn = USART1_IRQn, \
.dma_tx_ch = DMA1_Channel2 \
}

Copy link
Copy Markdown
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 \
}

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

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

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

Copy link
Copy Markdown
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