Покритикуйте код?

Adolf_Balalaykin
Offline
Зарегистрирован: 01.02.2021
  Доброго времени суток! Наваял таймер в качестве теста. Для удобства контроля вывел обратный отсчет времени на вэб панель. Я еще новичек и поэтому хотелось бы услышать конструктивную критику, а не просто облажать :). Критика с примерами была бы вообще очень к месту. Какие есть ошибки, замечания, что бы вы изменили ну и.т.д.
  Таймер будет работать на больших промежутках времени, а пока  для удобства отладки работаем в секундах. Хотел прикрутить какой нибудь конвертер времени. Что бы время конвертировать из секунд, в формат чч/мин/сек. Но пока ума не хватило как это сделать. Если есть идеи в этом направлении, буду очень признателен.
 
Sketch
#include <WiFi.h>
#include <AsyncTCP.h>
#include <ESPAsyncWebServer.h>
#include <SPIFFS.h>

// Задаем сетевые настройки
const char *ssid = "uname";
const char *password = "pass";

// На выводе GPIO25 по умолчанию устанавливаем 0.
bool ledState = false;

// Объявляем GPIO.
#define ledPin 25

AsyncWebServer server(80); // Запускаем асинхронный веб-сервер на 80 порту
AsyncWebSocket ws("/ws");  // Создаём объект, который будет обрабатывать websocket-ы:

// Переменные хранения времени (unsigned long)
uint32_t timer;

// Задаем время для таймеров
int32_t t1 = 5;    // Продолжительность работы таймера 5 сек.
int32_t t11 = 15; // Переодичность включения таймера 15 сек.

unsigned long previousMillis = 0;
const long interval = 1000; // Интервал между показаниями.

// Создаем Websocket сервер
void handleWebSocketMessage(void *arg, uint8_t *data, size_t len)
{
  AwsFrameInfo *info = (AwsFrameInfo *)arg;
  if (info->final && info->index == 0 && info->len == len && info->opcode == WS_TEXT)
  {
    data[len] = 0;
    if (strcmp((char *)data, "toggle") == 0) // Слушаем сообщения от js, если toggle = 0, то выполняем код ниже.
    {
      ledState = !ledState;         // Установим значение переменной ledState = 1, но если кнопка не нажата тогда ledState = 0
      ws.textAll(String(ledState)); // Уведомляем клиентов о переключении кнопки.
    }
  }
}

void onEvent(AsyncWebSocket *server, AsyncWebSocketClient *client, AwsEventType type, void *arg, uint8_t *data, size_t len)
{
  switch (type)
  {
  case WS_EVT_CONNECT: // когда клиент вошел в систему
    Serial.printf("WebSocket клиент #%u подключен с %s\n", client->id(), client->remoteIP().toString().c_str());
    break;
  case WS_EVT_DISCONNECT: // когда клиент вышел из системы
    Serial.printf("WebSocket клиент #%u отключен\n", client->id());
    break;
  case WS_EVT_DATA: // обработка полученных данных
    handleWebSocketMessage(arg, data, len);
    break;
  case WS_EVT_PONG:  // в ответ на запрос ping
  case WS_EVT_ERROR: // при получении ошибки от клиента
    break;
  }
}

// ----------------------------------------------------------------
// Инициализация
// ----------------------------------------------------------------

void initSPIFFS()
{
  if (!SPIFFS.begin())
  {
    Serial.println("При монтировании SPIFFS произошла ошибка");
    while (1)
      ;
  }
}

void initWiFi()
{
  WiFi.begin(ssid, password);
  Serial.print("Подключаемся к WiFi...");
  while (WiFi.status() != WL_CONNECTED)
  {
    Serial.print(".");
    delay(1000);
  }
  Serial.println();
  Serial.printf("Подключился к сети %s\n", ssid);
  Serial.print("ESP32 IP адрес: ");
  Serial.println(WiFi.localIP());
}

void initWebServer()
{
  server.begin();
}

void initWebSocket()
{
  ws.onEvent(onEvent);
  server.addHandler(&ws);
}

// Функция processor() заменяет заполнитель между символам %---%, в HTML-тексте состоянием ON/OFF.
String processor(const String &var)
{
  if (var == "STATE")
  {
    if (ledState)
    {
      return "ON";
    }
    else
    {
      return "OFF";
    }
  }
  return String();

//  Формируем остаток времени до запуска таймера
String timeLeft()
{
  int8_t IN10 = (t11 - (millis() / 1000L - timer));
  if (ledState)  // если таймер включен
  {
    return "ON"; // отправить статус "ON"
  }
  return String(IN10);
}

void setup()
{
  Serial.begin(115200);
  delay(1000);

  initSPIFFS();      // Инициализация SPIFFS
  initWiFi();            // Инициализация WiFi
  initWebServer(); // Инициализация Web сервера
  initWebSocket(); // Инициализация Websocket

  // Объявим GPIO выход (по умолчанию LOW)
  pinMode(ledPin, OUTPUT);
  digitalWrite(ledPin, LOW);

  // Маршрут до корневого каталога веб страницы
  server.on("/", HTTP_GET, [](AsyncWebServerRequest *request)
            { request->send(SPIFFS, "/index.html", "text/html", false, processor); });

  server.on("/script.js", HTTP_GET, [](AsyncWebServerRequest *request)
            { request->send(SPIFFS, "/script.js", "text/javascript"); });

  // Отправляем остаток времени до включения таймеров в js
  server.on("/time_Left", HTTP_GET, [](AsyncWebServerRequest *request)
            { request->send_P(200, "text/plain", timeLeft().c_str()); });
}

void loop()
{
  ws.cleanupClients();
  digitalWrite(ledPin, ledState);

  // Считываем каждую секунду.
  uint32_t currentMillis = millis();
  if (currentMillis - previousMillis >= interval)
  {

    if (millis() / 1000L - timer >= (ledState ? t1 : t11)) // t1-5 сек-продолжительность, t11-15 сек-периодичность
    {
      timer = millis() / 1000L;           // количество секунд с момента старта
      ledState = !ledState;
      digitalWrite(ledPin, ledState); // Подаем на GPIO25, высокий/низкий уровень
      ws.textAll(String(ledState));   // отправляем статус кнопки "ON"
    }
  }
}

Java Script

var gateway = `ws://${window.location.hostname}/ws`;
var websocket;
window.addEventListener('load', onLoad);

function initWebSocket() {
  console.log('Trying to open a WebSocket connection...');
  websocket = new WebSocket(gateway);
  websocket.onopen = onOpen;
  websocket.onclose = onClose;
  websocket.onmessage = onMessage;
}
function onOpen(event) {
  console.log('Connection opened');
}

function onClose(event) {
  console.log('Connection closed');
  setTimeout(initWebSocket, 2000);
}

function onMessage(event) {
  switch (event.data) {
		case '0': document.getElementById("state").innerHTML = "OFF"; document.getElementById('color').style.backgroundColor = "#c90411"; break
		case '1': document.getElementById("state").innerHTML = "ON &nbsp;"; document.getElementById('color').style.backgroundColor = "#04b50a"; break
  }
}

function onLoad(event) {
  initWebSocket();
  initButton();
}

HTML

<!DOCTYPE html>
<html lang="ru">

<head>
	<meta charset="UTF-8" , name="viewport" content="width=device-width, initial-scale=1.0">
</head>

<body>
	<br>
	&nbsp; GPIO25 &nbsp; <button id="color"><span id="state">%STATE%</span></button>&nbsp; &nbsp; до включения осталось...
	<span id="dateTime"></span>
	<script src="script.js"></script>
</body>

</html>

 

 

BOOM
BOOM аватар
Offline
Зарегистрирован: 14.11.2018

1. У тебя почему разные типы используются? Например, "unsigned long" и "uint32_t". Из каких таких "предпочтений" ты так делаешь?

2. У тебя интервалы могут быть отрицательными (23 и 24 строки)? Могут меняться в самой программе?

Короче, я далеко не профессионал, но с типа данных тут точно поработать нужно. Причем сначала в голове ))

3. В html пишешь:

<span id="dateTime"></span>

А где ты с данным id в ява-скрипт работаешь? Не вижу... Оно вообще работает?

DetSimen
DetSimen аватар
Offline
Зарегистрирован: 25.01.2017

Adolf_Balalaykin пишет:
Покритикуйте код

Код - авно. Достаточно?

Adolf_Balalaykin
Offline
Зарегистрирован: 01.02.2021

BOOM пишет:

1. У тебя почему разные типы используются? Например, "unsigned long" и "uint32_t". Из каких таких "предпочтений" ты так делаешь?

2. У тебя интервалы могут быть отрицательными (23 и 24 строки)? Могут меняться в самой программе?

Короче, я далеко не профессионал, но с типа данных тут точно поработать нужно. Причем сначала в голове ))

3. В html пишешь:

<span id="dateTime"></span>

А где ты с данным id в ява-скрипт работаешь? Не вижу... Оно вообще работает?

1. Один и тот же тип данных. uint32_t, это альтернативное название. Диаппазон у них одинаков. Предпочтений нет, что первым в голову пришло то и прописал. Можно и к одному виду привести, для красоты ).

2. О, точняк! Поменяю на uint32_t.

3. Ты смотри, заметил )) На месте он. Функция установки интервала, 5 строка. В первом сообщении js код не полный привел. Опять же невнимательность.

  setInterval(function () {
    var xhttp = new XMLHttpRequest();
    xhttp.onreadystatechange = function () {
      if (this.readyState == 4 && this.status == 200) {
        document.getElementById("dateTime").innerHTML = this.responseText;
      }
    };
    xhttp.open("GET", "/timeLeft", true);
    xhttp.send();
  }, 1000);

Спасибо Бум!

DetSimen пишет:

Adolf_Balalaykin пишет:
Покритикуйте код

Код - авно. Достаточно?

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

BOOM
BOOM аватар
Offline
Зарегистрирован: 14.11.2018

Adolf_Balalaykin пишет:

BOOM пишет:

1. У тебя почему разные типы используются? Например, "unsigned long" и "uint32_t". Из каких таких "предпочтений" ты так делаешь?

2. У тебя интервалы могут быть отрицательными (23 и 24 строки)? Могут меняться в самой программе?

Короче, я далеко не профессионал, но с типа данных тут точно поработать нужно. Причем сначала в голове ))

3. В html пишешь:

<span id="dateTime"></span>

А где ты с данным id в ява-скрипт работаешь? Не вижу... Оно вообще работает?

1. Один и тот же тип данных. uint32_t, это альтернативное название. Диаппазон у них одинаков. Предпочтений нет, что первым в голову пришло то и прописал. Можно и к одному виду привести, для красоты ).

2. О, точняк! Поменяю на uint32_t.

3. Ты смотри, заметил )) На месте он. Функция установки интервала, 5 строка. В первом сообщении js код не полный привел. Опять же невнимательность.

  setInterval(function () {
    var xhttp = new XMLHttpRequest();
    xhttp.onreadystatechange = function () {
      if (this.readyState == 4 && this.status == 200) {
        document.getElementById("dateTime").innerHTML = this.responseText;
      }
    };
    xhttp.open("GET", "/timeLeft", true);
    xhttp.send();
  }, 1000);

Спасибо Бум!

DetSimen пишет:

Adolf_Balalaykin пишет:
Покритикуйте код

Код - авно. Достаточно?

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

1. Да не совсем, иначе я бы тебе это не писал. Погугли. 

3. Что увидел - то и написал (хотя в код мк не вчитывался).

ЗЫ: Дед говорит малословно, он уже устал. А я ещё молодой (вторая молодость, так чтоль говорится?!), я ещё чего-то говорю «по шире» )))

DetSimen
DetSimen аватар
Offline
Зарегистрирован: 25.01.2017

Adolf_Balalaykin пишет:

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

Дак ты этого не просил.  Я сделал  ровно то, что ты просил, покритиковал код.

mykaida
mykaida аватар
Offline
Зарегистрирован: 12.07.2018

Слишком большой. Нас учили, что каждую программу можно уменьшить минимум на один оператор. Добивайтесь идеала!

v258
v258 аватар
Offline
Зарегистрирован: 25.05.2020

mykaida пишет:

Слишком большой. Нас учили, что каждую программу можно уменьшить минимум на один оператор. Добивайтесь идеала!

Работает - не трогай )))

mykaida
mykaida аватар
Offline
Зарегистрирован: 12.07.2018

v258 пишет:

Работает - не трогай )))

И энто тоже!

Если скорости хватает.

mykaida
mykaida аватар
Offline
Зарегистрирован: 12.07.2018

А вот под это я расскажу вам МИФИческую байку;

Надо было посчитать реактор в реальном времени. США пощло по простому пути - построили крутой компьютер, а в  СССР от бедности, написали крутую программу. 

Учите матаппарат.

mykaida
mykaida аватар
Offline
Зарегистрирован: 12.07.2018

Кстати на FORTRANe

qwone
qwone аватар
Offline
Зарегистрирован: 03.07.2016

mykaida пишет:

Слишком большой. Нас учили, что каждую программу можно уменьшить минимум на один оператор. Добивайтесь идеала!

Вы бы еще писание на глине приплели. Тогда не было сильного компилятора. Сейчас идут другие "погремушки".

Morroc
Offline
Зарегистрирован: 24.10.2016

mykaida пишет:

Нас учили, что каждую программу можно уменьшить минимум на один оператор. 

Она же так схлопнется и Землю в черную дыру затянет )

andriano
andriano аватар
Offline
Зарегистрирован: 20.06.2015

mykaida пишет:

Слишком большой. Нас учили, что каждую программу можно уменьшить минимум на один оператор. Добивайтесь идеала!

Вопрос в другом: имеет ли это хоть какой-нибудь смысл, если писать на языке отличном от Ассемблера?

nik182
Offline
Зарегистрирован: 04.05.2015

mykaida пишет:

Слишком большой. Нас учили, что каждую программу можно уменьшить минимум на один оператор. Добивайтесь идеала!

Ты упустил слово работающую. В конце концов наступает момент когда выкидывание оператора приводит к коллапсу. Предыдущая версия и будет близка к идеалу.