Будут ли авторам прошивки интересны мысли и предложения по улучшению организации кода?
Самый простой пример - как минимум четыре, а как максимум 11 строчек из метода MAIN про knock/ДД должны бы переехать в метод knock_init (а еще лучше - init_knock), а метод этот должен переехать в файл knock.c
Ну и вся инициализация портов могла бы выделиться в метос init_ports, аналогично - метод init_sensors
И так далее.
На следующем уровне - было бы хорошо чуть отодвинуть низкоуровневый код именно этого микроконтроллера от кода уровня логики. Потенциально можно сделать директорию, весь код в которой не будет знать ничего о конкретной аппаратуре контроллера - отделить на уровне папок. Всё-таки 70 файлов в одной папке - немного жёстко.
Если Вы принимаете патчи - я могу оформить часть этих идей в виде патча.
Про исходные коды - мысли о рефакторинге
-
- 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: про исходные коды - мысли о рефакторинге
Я думаю это было бы очень удобно при портирования на другие МК.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-покаления
https://github.com/denami/secu3droid
Renault Laguna 2.0 i.e.
Opel Zafira A 2.2 i.e. , ГБО 4-покаления
- 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: про исходные коды - мысли о рефакторинге
Несомненно.Будут ли авторам прошивки интересны мысли и предложения по улучшению организации кода?
Вы можете зарегистрироваться на github, сделать ответвление и там сделать рефакторинг, а я после проверки могу перенести ваши изменения в master.Если Вы принимаете патчи - я могу оформить часть этих идей в виде патча.
Можно, но лучше назвать knock_init, так как я придерживаюсь правила: сначала название модуля или функционала а потом название действия. Кстати, порядок вызова функций может иметь значение, так что нужно это делать очень осторожно и все 10 раз перепроверять и обдумывать.Самый простой пример - как минимум четыре, а как максимум 11 строчек из метода MAIN про knock/ДД должны бы переехать в метод knock_init (а еще лучше - init_knock), а метод этот должен переехать в файл knock.c
Полностью это не получится сделать из-за накладных расходов, но частично можно. В любом случае, если вы считаете что у вас получится лучше, вы можете попробовать, а я потом прийму решение переносить ваши изменения в master или нет.На следующем уровне - было бы хорошо чуть отодвинуть низкоуровневый код именно этого микроконтроллера от кода уровня логики. Потенциально можно сделать директорию, весь код в которой не будет знать ничего о конкретной аппаратуре контроллера - отделить на уровне папок. Всё-таки 70 файлов в одной папке - немного жёстко.
Уже сделано.Ну и вся инициализация портов могла бы выделиться в метос 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 (вступаем!)
Клуб проекта в Facebook https://www.facebook.com/groups/secu3club
Клуб проекта ВКонтакте https://vk.com/secu3club (вступаем!)
Re: про исходные коды - мысли о рефакторинге
о! вроде бы разобрался со всеми этими pull/commit/push/... Смотрите
- 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: про исходные коды - мысли о рефакторинге
Да. Вижу 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 (вступаем!)
Клуб проекта в Facebook https://www.facebook.com/groups/secu3club
Клуб проекта ВКонтакте https://vk.com/secu3club (вступаем!)
Re: про исходные коды - мысли о рефакторинге
Угу, я даже сделал Pull Request - посмотрите пожалуйста, когда будет время.STC wrote:Да. Вижу abelom / secu3app
(мелкие вопросы, но просто чтоб понять как Вам видется правильнее для вашего проекта):
"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
- 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: про исходные коды - мысли о рефакторинге
По поводу макроса я подумаю, пока просто занят немного другими вещами.
В ATMega16 прошивка уже не помещается, вот вам линковщик и выдает ошибку. Для ATMega64 компилировать нет смысла, так как оно работать не будет. В будущем планируется еще добавить ATmega644.
В 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 (вступаем!)
Клуб проекта в Facebook https://www.facebook.com/groups/secu3club
Клуб проекта ВКонтакте https://vk.com/secu3club (вступаем!)