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

Обсуждаем прошивки, алгоритмы работы в разных режимах (например, алгоритм работы автоподсоса).
Наборы тюнингованых прошивок МПСЗ SECU (заточенных под конкретную конфигурацию двигателя)
Post Reply
andrey
Posts: 7
Joined: 25 Jun 2013, 18:15
Been thanked: 2 times

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

Post by andrey »

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

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

Если Вы принимаете патчи - я могу оформить часть этих идей в виде патча.
denami
Posts: 209
Joined: 08 May 2013, 23:58
Your CAR: Opel Zafira A 2.2i
SECU version: official SECU-3T
Location: Gdansk Poland
Has thanked: 20 times
Been thanked: 46 times

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

Post by denami »

andrey wrote:На следующем уровне - было бы хорошо чуть отодвинуть низкоуровневый код именно этого микроконтроллера от кода уровня логики. Потенциально можно сделать директорию, весь код в которой не будет знать ничего о конкретной аппаратуре контроллера - отделить на уровне папок. Всё-таки 70 файлов в одной папке - немного жёстко.
Я думаю это было бы очень удобно при портирования на другие МК.
https://github.com/denami/secu3_blueloger
https://github.com/denami/secu3droid
Renault Laguna 2.0 i.e.
Opel Zafira A 2.2 i.e. , ГБО 4-покаления
User avatar
STC
Posts: 13843
Joined: 30 Apr 2013, 23:41
Your CAR: AZLK 2140
SECU version: DIY SECU-3
Location: North Korea
Has thanked: 2160 times
Been thanked: 4335 times
Contact:

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

Post by STC »

Будут ли авторам прошивки интересны мысли и предложения по улучшению организации кода?
Несомненно.
Если Вы принимаете патчи - я могу оформить часть этих идей в виде патча.
Вы можете зарегистрироваться на 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
Posts: 7
Joined: 25 Jun 2013, 18:15
Been thanked: 2 times

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

Post by andrey »

о! вроде бы разобрался со всеми этими pull/commit/push/... Смотрите
User avatar
STC
Posts: 13843
Joined: 30 Apr 2013, 23:41
Your CAR: AZLK 2140
SECU version: DIY SECU-3
Location: North Korea
Has thanked: 2160 times
Been thanked: 4335 times
Contact:

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

Post by STC »

Да. Вижу 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
Posts: 7
Joined: 25 Jun 2013, 18:15
Been thanked: 2 times

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

Post by andrey »

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 собирается. Я что-то делаю не так?
User avatar
STC
Posts: 13843
Joined: 30 Apr 2013, 23:41
Your CAR: AZLK 2140
SECU version: DIY SECU-3
Location: North Korea
Has thanked: 2160 times
Been thanked: 4335 times
Contact:

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

Post by STC »

По поводу макроса я подумаю, пока просто занят немного другими вещами.
В 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 (вступаем!)
Post Reply

Return to “Прошивки”