Комментарии 15
Ох и сурово же Вы подключили карточку. Не будет ли ей плохо от 5 вольт? Обычно питают хотя бы от 3.3 и на шину ставят резистивные делители, а лучше даже буфера, которые 5 в 3.3 преобразуют и обратно. Или будет использоваться бандл, где уже всё это на плате есть?
+2
Представленный пример кода (причем вроде как учебный) демонстрирует огромное количество антипаттернов и просто гадких практик.
В целом, применение всех этих исправлений должно раза так в три-четыре сократить исходник, увеличить его читаемость и надежность.
- Тотальное отсутствие комментариев. В учебном коде для аппаратной платформы, где за каждым интересным присваиванием служебных регистров придется лезть в даташит, комментарии со ссылками на даташит обязательны
- Дикое количество ненужных глобальных переменных с бессмысленными именами в дельфийском стиле (_bounseInput9S, _bounseInput8S, ...). Глобальные переменные оправданы, только если к ним есть обращение из обработчиков прерываний. Наличие кучи таких переменных порождает неочевидный, неуправляемый код с сильной внутренней связностью — просто так взять его кусок и обозреть — оттестировать отдельно будет просто невозможно.
Причем эти глобальные переменные трогательно помечены подчеркиванием, видимо для отличия от локальных переменных. Любая современная IDE и так умеет их раскрашивать разным цветом. Тут только венгерской нотации не хватает до полной обфускации кода.
- «Китайский стиль»
_bounseInput9O = digitalRead(9); _bounseInput6O = digitalRead(6); _bounseInput4O = digitalRead(4); _bounseInput5O = digitalRead(5);
Запись:
static const byte index[]={4,5,6,9}; for(size_t i=0;i<sizeof(index);i++) { bounceio[index[i]]=digitalRead(index[i]); }
В любом случае скорее всего развернется оптимизатором в написанное выше, но при этом защитит автора от опечаток при копировании-вставке.
ОТСТУПЛЕНИЕ
С другой стороны, это камень в огород разработчиков ардуинской библиотеки. Уж в C++ можно было скалькировать хотя бы потоковый ввод-вывод в духе cin и cout. Тогда было бы можно творить конструкции в духе:
GPIO<<pinRange(5,6)<<pin(9)<<modeIN<<LevelHi;
- Чудесатые условия:
if (1) { _dispTempLength1 = (((((String((_tRTC1.hour())))) + (String(":")) + ((String((_tRTC1.minute())))) + (String(":")) + ((String((_tRTC1.second()))))))).length(); if (_disp1oldLength > _dispTempLength1) { _isNeedClearDisp1 = 1; } _disp1oldLength = _dispTempLength1; _lcd1.setCursor(0, 0); _lcd1.print(((((String((_tRTC1.hour())))) + (String(":")) + ((String((_tRTC1.minute())))) + (String(":")) + ((String((_tRTC1.second()))))))); } else { if (_disp1oldLength > 0) { _isNeedClearDisp1 = 1; _disp1oldLength = 0; } }
Вызова содержимого блока else мы здесь будем ждать вечно. Если оно «так и задумано», зачем здесь блок else?
Какая-нибудь PVS Studio должна была бы здесь выдать в духе:
WARNING 100500: The cat on keyboard!
- -Оператор множественного выбора?
-Не, не знаю такого…
if ((_gtv8) == 0) { _mux1 = String("DS18B2"); } if ((_gtv8) == 1) { _mux1 = String("Bmp-085"); } if ((_gtv8) == 2) { _mux1 = String("DHT-22"); }
Стоит все же писать хотя бы как:
switch(_mux1) { case 0: { _mux1 = String("DS18B2"); break; } case 1: { _mux1 = String("Bmp-085"); break; } case 2: { _mux1 = String("DHT-22"); break; } }
Ба-тю-шки, на самом деле, здесь даже switch-case не нужен, потому как это все прекрасно заменяется на подстановку из массива:
static const char* subst[]={"DS18B2","Bmp-085","DHT-22"}; // подстановка названия датчика _mux1 = String(subst[_gtv8]);
- И еще одно место для замены на подстановку:
if (cfg == 0x00) raw = raw & ~7; else if (cfg == 0x20) raw = raw & ~3; else if (cfg == 0x40) raw = raw & ~1;
Если мы поделим аргумент cfg (0,32,64) на 32, мы получим 0, 1, 2 — прекрасный индекс для подстановки. Понятно, что деление на степень двойки нужно осуществлять сдвигом на 5 бит вправо:
//Если мы поделим аргумент cfg (0,32,64) на 32, мы получим 0, 1, 2 - прекрасный индекс //для подстановки. Понятно, что деление на степень двойки нужно осуществлять сдвигом на 5 бит вправо: static byte subst[]={~7,~3,~1}; //А здесь комментарий про то, зачем нам эти маски raw&=subst[cfg>>5];
В целом, применение всех этих исправлений должно раза так в три-четыре сократить исходник, увеличить его читаемость и надежность.
+2
Сократить исходник в 3-4 раза? раз в 6-8 после первого взгляда.
0
Я Вами просто восхищаюсь! Просмотрели огромный код, нашли кучу ошибок. И какой — же я гений что смог написать этот нечитаемый код, запомнить что означают эти бредовые переменные. Вы прям и меня в собственных глазах подняли.
А может Вам все таки стоило посмотреть хотя бы пять минут начала урока и пять минут конца урока. Тогда бы все стало понятно. Код сформирован автоматически, с помощью графической среды программирования. Пользователь его не видит, так что ни названия переменных, ни глубина вложенности, ни правила форматирования роли не играет. Как и впрочем возможность модификации и поддержки. В случае необходимости все изменения производятся в графической среде и генерится новый код.
Я не утверждаю что компилятор идеален, конечно еще много возможностей для оптимизации. Все таки проекту еще и года нет. Но все таки прежде чем писать такие разгромные комментарии было — бы неплохо разобраться в вопросе. Тут же как говорится за версту видно что код автоматически сгенерён.
А может Вам все таки стоило посмотреть хотя бы пять минут начала урока и пять минут конца урока. Тогда бы все стало понятно. Код сформирован автоматически, с помощью графической среды программирования. Пользователь его не видит, так что ни названия переменных, ни глубина вложенности, ни правила форматирования роли не играет. Как и впрочем возможность модификации и поддержки. В случае необходимости все изменения производятся в графической среде и генерится новый код.
Я не утверждаю что компилятор идеален, конечно еще много возможностей для оптимизации. Все таки проекту еще и года нет. Но все таки прежде чем писать такие разгромные комментарии было — бы неплохо разобраться в вопросе. Тут же как говорится за версту видно что код автоматически сгенерён.
-1
И ещё безумная стилистика и рыхлость кода. То у вас фигурная скобка на одной строке с if то на следующей… Чувство что код слеплен из каких-то разрозненных кусков.
0
Ну так примерно и есть. Обрабатываются логические цепочки, и формируется код из заготовок, с подстановкой значений. Ну это в общем плане, все конечно намного сложнее. Ну и вообще зачем форматирование и визуальная красота. я мог все вывести вообще в одну строку и ничего бы не изменилось. Как я уже писал ваше, пользователь этого кода не видит. У меня вообще в планах отказаться от Arduino IDE, и скармливать код напрямую компилятору через командную строку
0
Какой объем данных вам нужно хранить? Может стоит использовать вместо SD карты просто ПЗУ (EEPROM)? Попробуйте что-нибудь вроде AT24C1024. Подключается также через SPI.
0
У меня (1.0.6):
sketch_jan22a:9: error: 'BMP085' does not name a type
sketch_jan22a:22: error: 'OneWire' does not name a type
sketch_jan22a.ino: In function 'void setup()':
sketch_jan22a:79: error: '_bmp085' was not declared in this scope
sketch_jan22a:79: error: 'MODE_ULTRA_HIGHRES' was not declared in this scope
sketch_jan22a.ino: In function 'void loop()':
sketch_jan22a:122: error: '_bmp085' was not declared in this scope
sketch_jan22a.ino: In function 'float _readDS18_ow3(byte*, byte)':
sketch_jan22a:647: error: '_ow3' was not declared in this scope
Что делаю не так? Заранее благодарен за ответ.
sketch_jan22a:9: error: 'BMP085' does not name a type
sketch_jan22a:22: error: 'OneWire' does not name a type
sketch_jan22a.ino: In function 'void setup()':
sketch_jan22a:79: error: '_bmp085' was not declared in this scope
sketch_jan22a:79: error: 'MODE_ULTRA_HIGHRES' was not declared in this scope
sketch_jan22a.ino: In function 'void loop()':
sketch_jan22a:122: error: '_bmp085' was not declared in this scope
sketch_jan22a.ino: In function 'float _readDS18_ow3(byte*, byte)':
sketch_jan22a:647: error: '_ow3' was not declared in this scope
Что делаю не так? Заранее благодарен за ответ.
0
Подскажите пожалуйста, вы ArduinoIDE какое используете? Скетч рассчитан на версию с установленными библиотеками.
Вот ссылка на архив с нужными библиотеками:Библиотеки
Остальные по моему в стандартной установке ArduinoIDE. А вообще советую скачать программу с сайта flprog.ru и загрузить файл проекта. Так же скорее всего надо будет изменить адрес дисплея на тот который прошит в вашем дисплее. В программе есть инструменты для определения вашего адреса. Так же вместе с программой ставится Arduino IDE со всеми необходимыми библиотеками
#include <Wire.h>
#include «DHT.h»
#include <LiquidCrystal_I2C.h>
#include <SPI.h>
#include <SD.h>
#include <OneWire.h>
#include «RTClib.h»
#include <BMP085.h>
Вот ссылка на архив с нужными библиотеками:Библиотеки
Остальные по моему в стандартной установке ArduinoIDE. А вообще советую скачать программу с сайта flprog.ru и загрузить файл проекта. Так же скорее всего надо будет изменить адрес дисплея на тот который прошит в вашем дисплее. В программе есть инструменты для определения вашего адреса. Так же вместе с программой ставится Arduino IDE со всеми необходимыми библиотеками
0
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Метеостанция на прокачку. Добавим к Ардуине SD карту