American English British English Български Čeština Français Deutsch (Du) Magyar Italiano 日本語 Lietuvių Polski Српски Slovenčina Español (Tú) Türkçe Українська
Select language:

Про исходные коды - мысли о рефакторинге

<<

andrey

User avatar

Posts: 7

Joined: 25 Jun 2013, 18:15


Has thanked: 0 time
Been thanked: 2 times

Post 25 Jun 2013, 20:11

Про исходные коды - мысли о рефакторинге

Будут ли авторам прошивки интересны мысли и предложения по улучшению организации кода?
Самый простой пример - как минимум четыре, а как максимум 11 строчек из метода MAIN про knock/ДД должны бы переехать в метод knock_init (а еще лучше - init_knock), а метод этот должен переехать в файл knock.c
Ну и вся инициализация портов могла бы выделиться в метос init_ports, аналогично - метод init_sensors
И так далее.

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

Если Вы принимаете патчи - я могу оформить часть этих идей в виде патча.
<<

denami

User avatar

Posts: 115

Joined: 08 May 2013, 23:58

Location: Поставы РБ

Your CAR: Fiat Tipo

SECU version: official SECU-3T


Has thanked: 1 time
Been thanked: 15 times

Post 25 Jun 2013, 21:37

Re: про исходные коды - мысли о рефакторинге

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


Я думаю это было бы очень удобно при портирования на другие МК.
https://github.com/denami/secu3_blueloger
Fiat Tipo 2.0 i.e. ECU IAW : SECU-3t rev6 Bluetooth в процессе установки.
Renault Laguna 2.0 i.e.
Mazda 626 GF 1.8 i.e. , ГБО 5-покаления
<<

STC

User avatar

Posts: 6557

Joined: 30 Apr 2013, 23:41

Location: Ukraine

Your CAR: AZLK 2140

SECU version: DIY SECU-3


Has thanked: 804 times
Been thanked: 1321 times

Post 26 Jun 2013, 12:47

Re: про исходные коды - мысли о рефакторинге

Будут ли авторам прошивки интересны мысли и предложения по улучшению организации кода?

Несомненно.
Если Вы принимаете патчи - я могу оформить часть этих идей в виде патча.

Вы можете зарегистрироваться на github, сделать ответвление и там сделать рефакторинг, а я после проверки могу перенести ваши изменения в master.
Самый простой пример - как минимум четыре, а как максимум 11 строчек из метода MAIN про knock/ДД должны бы переехать в метод knock_init (а еще лучше - init_knock), а метод этот должен переехать в файл knock.c

Можно, но лучше назвать knock_init, так как я придерживаюсь правила: сначала название модуля или функционала а потом название действия. Кстати, порядок вызова функций может иметь значение, так что нужно это делать очень осторожно и все 10 раз перепроверять и обдумывать.

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

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

Ну и вся инициализация портов могла бы выделиться в метос init_ports

Уже сделано.
Author of the SECU-3™ project http://SECU-3.org. An open source engine control unit / Ignition control system, (C) 2007.
Клуб проекта в Facebook https://www.facebook.com/groups/secu3club
Клуб проекта ВКонтакте https://vk.com/secu3club (вступаем!)
<<

andrey

User avatar

Posts: 7

Joined: 25 Jun 2013, 18:15


Has thanked: 0 time
Been thanked: 2 times

Post 26 Jun 2013, 15:42

Re: про исходные коды - мысли о рефакторинге

о! вроде бы разобрался со всеми этими pull/commit/push/... Смотрите
<<

STC

User avatar

Posts: 6557

Joined: 30 Apr 2013, 23:41

Location: Ukraine

Your CAR: AZLK 2140

SECU version: DIY SECU-3


Has thanked: 804 times
Been thanked: 1321 times

Post 26 Jun 2013, 16:14

Re: про исходные коды - мысли о рефакторинге

Да. Вижу abelom / secu3app
Author of the SECU-3™ project http://SECU-3.org. An open source engine control unit / Ignition control system, (C) 2007.
Клуб проекта в Facebook https://www.facebook.com/groups/secu3club
Клуб проекта ВКонтакте https://vk.com/secu3club (вступаем!)
<<

andrey

User avatar

Posts: 7

Joined: 25 Jun 2013, 18:15


Has thanked: 0 time
Been thanked: 2 times

Post 26 Jun 2013, 18:33

Re: про исходные коды - мысли о рефакторинге

STC wrote:Да. Вижу abelom / secu3app

Угу, я даже сделал Pull Request - посмотрите пожалуйста, когда будет время.

(мелкие вопросы, но просто чтоб понять как Вам видется правильнее для вашего проекта):

"d->sens.frequen4 > d->param.starter_off" из knklogic_detect мне кажется должно быть выделенно в метод/макрос с говорящим названием типа "isEngineRunning(struct ecudata_t* d)". Тем более, что это проверка - а это всё-таки логика, не имеющая прямого отношения к детонации - продублированна уже в другом месте, в starter_control. Значит метод/макрос для начала можно положить в starter.c

Вопрос - есть ли возражения против метода? Или делать макросом?

Продолжая про knklogic_detect: а зачем нужен knock_flag как отдельный флаг/переменная? Не будет ли проще сделать метод isKnocking - а дальше использовать этот метод два раза, один раз из knklogic_detect для установки-снятия ошибки, и второй раз из knklogic_retard для собсвенно действия? Будет на один флаг меньше, программа будет чуть-чуть проще в плане неявной зависимости между порядком исполнения методов.

PS: я скачал WinAVR 2010 и у меня make видимо из cygwin, у меня M16 не линкуется ошибкой "section .firmware_data [00003134 -> 00003dff] overlaps section .text [00000000 -> 0000595b]" а M64 не компилируется ошибкой "sources/ckps.c:791: error: 'UCSRB' undeclared" - только M32 собирается. Я что-то делаю не так?
<<

STC

User avatar

Posts: 6557

Joined: 30 Apr 2013, 23:41

Location: Ukraine

Your CAR: AZLK 2140

SECU version: DIY SECU-3


Has thanked: 804 times
Been thanked: 1321 times

Post 27 Jun 2013, 10:07

Re: про исходные коды - мысли о рефакторинге

По поводу макроса я подумаю, пока просто занят немного другими вещами.
В ATMega16 прошивка уже не помещается, вот вам линковщик и выдает ошибку. Для ATMega64 компилировать нет смысла, так как оно работать не будет. В будущем планируется еще добавить ATmega644.
Author of the SECU-3™ project http://SECU-3.org. An open source engine control unit / Ignition control system, (C) 2007.
Клуб проекта в Facebook https://www.facebook.com/groups/secu3club
Клуб проекта ВКонтакте https://vk.com/secu3club (вступаем!)

Return to Прошивки

Who is online

Users browsing this forum: No registered users and 2 guests

cron
Powered by phpBB® Forum Software © phpBB Group.