Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Hinweis zu app::loop Optimierung #536

Closed
beegee3 opened this issue Dec 31, 2022 · 4 comments
Closed

Feature Request: Hinweis zu app::loop Optimierung #536

beegee3 opened this issue Dec 31, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request fixed dev fixed

Comments

@beegee3
Copy link
Contributor

beegee3 commented Dec 31, 2022

@lumapu: kleine Anpassung zur Entlastung der app::loop
neue Variable bool mRxRdy mit Default mRxRdy = false;
und in der loop

if (ah::checkTicker(&mRxTicker, 5)) {
        ...
        if (rxRdy && !mRxRdy)
            mPayload.process(true, mConfig->nrf.maxRetransPerPyld, &mStat);
        mRxRdy = rxRdy;
}

dann wird mPayload.process nur noch bei Bedarf aufgerufen, nicht mehr alle 5ms !

@lumapu
Copy link
Owner

lumapu commented Jan 1, 2023

Ich glaube das funktioniert nicht und halbiert eher nur den Takt. Besser wäre es evtl. direkt in der payload.h einzubauen, zB. bei add ein mNewData = true und bei process eine Prüfung if(!mNewData) return.
Aber auch das funktioniert nicht ohne weiteres, da in die Payload noch zu viel 'reingestrickt' ist. Hier muss ich zuerst aufräumen und die ganzen Control Commands auslagern, zB. Richtung sendTicker in der app.cpp

@beegee3
Copy link
Contributor Author

beegee3 commented Jan 2, 2023

Du hast ja jetzt angefangen, den ganzen payload Komplex zu überarbeiten, scheint aber im Moment zu viel des Guten zu sein. Bekomme keine Verbindung zum Inverter, erhalte im serial log für alle sendTimePacket x (x = 1, 5, b) und Chyy (yy = 03, 23, 40, 61, 75) nur:
W: while retrieving data: last frame missing: Request Complete Retransmit
I: (#0) sendTimePacket
I: sendTimePacket x
I: TX 27B Chyy | ...

@beegee3
Copy link
Contributor Author

beegee3 commented Jan 2, 2023

Oh sorry, war noch auf v.05.67, gibt ja schon v.05.68, ich schau nochmal ...

... ja, jetzt geht's, nochmals sorry!

@stefan123t
Copy link
Collaborator

@lumapu habe auch die Tage mal im payload Komplex herum gestochert. Leider noch basierend auf der 0.5.64:

git diff on top of 0.5.64

$ git diff
diff --git a/src/app.cpp b/src/app.cpp
index 29df1e6..49eec17 100644
--- a/src/app.cpp
+++ b/src/app.cpp
@@ -217,15 +217,15 @@ void app::tickSend(void) {
 
                 if (iv->devControlRequest) {
                     if (mConfig->serial.debug)
-                        DPRINTLN(DBG_INFO, F("(#") + String(iv->id) + F(") Devcontrol request ") + String(iv->devControlCmd) + F(" power limit ") + String(iv->powerLimit[0]));
-                    mSys->Radio.sendControlPacket(iv->radioId.u64, iv->devControlCmd, iv->powerLimit);
+                        DPRINTLN(DBG_INFO, F("(#") + String(iv->id) + F(") requestDevControlPacket ") + String(iv->devControlCmd) + F(" power limit ") + String(iv->powerLimit[0]));
+                    mSys->Radio.requestDevControlPacket(iv->radioId.u64, iv->devControlCmd, iv->powerLimit);
                     mPayload.setTxCmd(iv, iv->devControlCmd);
                     iv->clearCmdQueue();
                     iv->enqueCommand<InfoCommand>(SystemConfigPara); // read back power limit
                 } else {
                     uint8_t cmd = iv->getQueuedCmd();
-                    DPRINTLN(DBG_INFO, F("(#") + String(iv->id) + F(") sendTimePacket"));
-                    mSys->Radio.sendTimePacket(iv->radioId.u64, cmd, mPayload.getTs(iv), iv->alarmMesIndex);
+                    DPRINTLN(DBG_INFO, F("(#") + String(iv->id) + F(") requestDevInfoPacket"));
+                    mSys->Radio.requestDevInfoPacket(iv->radioId.u64, cmd, mPayload.getTs(iv), iv->alarmMesIndex);
                     mPayload.setTxCmd(iv, cmd);
                     mRxTicker = 0;
                 }
@@ -242,7 +242,6 @@ void app::tickSend(void) {
 
 //-----------------------------------------------------------------------------
 void app::handleIntr(void) {
-    DPRINTLN(DBG_VERBOSE, F("app::handleIntr"));
     mSys->Radio.handleIntr();
 }
 
diff --git a/src/hm/hmRadio.h b/src/hm/hmRadio.h
index 13508a4..f64c702 100644
--- a/src/hm/hmRadio.h
+++ b/src/hm/hmRadio.h
@@ -32,7 +32,7 @@
 #define RF_CHANNELS             5
 #define RF_LOOP_CNT             300
 
-#define TX_REQ_INFO         0x15
+#define TX_REQ_DEVINFO      0x15
 #define TX_REQ_DEVCONTROL   0x51
 #define ALL_FRAMES          0x80
 #define SINGLE_FRAME        0x81
@@ -194,9 +194,9 @@ class HmRadio {
             return mRfChLst[mTxChIdx];
         }
 
-        void sendControlPacket(uint64_t invId, uint8_t cmd, uint16_t *data) {
-            DPRINTLN(DBG_INFO, F("sendControlPacket cmd: ") + String(cmd));
-            sendCmdPacket(invId, TX_REQ_DEVCONTROL, SINGLE_FRAME, false);
+        void requestDevControlPacket(uint64_t invId, uint8_t cmd, uint16_t *data) {
+            DPRINTLN(DBG_INFO, F("requestDevControlPacket cmd: ") + String(cmd));
+            requestMainCmdPacket(invId, TX_REQ_DEVCONTROL, SINGLE_FRAME, false);
             uint8_t cnt = 0;
             mTxBuf[10 + cnt++] = cmd; // cmd -> 0 on, 1 off, 2 restart, 11 active power, 12 reactive power, 13 power factor
             mTxBuf[10 + cnt++] = 0x00;
@@ -218,9 +218,9 @@ class HmRadio {
             sendPacket(invId, mTxBuf, 10 + cnt + 1, true);
         }
 
-        void sendTimePacket(uint64_t invId, uint8_t cmd, uint32_t ts, uint16_t alarmMesId) {
-            DPRINTLN(DBG_INFO, F("sendTimePacket ") + String(cmd, HEX));
-            sendCmdPacket(invId, TX_REQ_INFO, ALL_FRAMES, false);
+        void requestDevInfoPacket(uint64_t invId, uint8_t cmd, uint32_t ts, uint16_t alarmMesId) {
+            DPRINTLN(DBG_INFO, F("requestDevInfoPacket ") + String(cmd, HEX));
+            requestMainCmdPacket(invId, TX_REQ_DEVINFO, ALL_FRAMES, false);
             mTxBuf[10] = cmd; // cid
             mTxBuf[11] = 0x00;
             CP_U32_LittleEndian(&mTxBuf[12], ts);
@@ -236,8 +236,8 @@ class HmRadio {
             sendPacket(invId, mTxBuf, 27, true);
         }
 
-        void sendCmdPacket(uint64_t invId, uint8_t mid, uint8_t pid, bool calcCrc = true) {
-            DPRINTLN(DBG_VERBOSE, F("sendCmdPacket, mid: ") + String(mid, HEX) + F(" pid: ") + String(pid, HEX));
+        void requestMainCmdPacket(uint64_t invId, uint8_t mid, uint8_t pid, bool calcCrc = true) {
+            DPRINTLN(DBG_VERBOSE, F("requestMainCmdPacket, mid: ") + String(mid, HEX) + F(" pid: ") + String(pid, HEX));
             memset(mTxBuf, 0, MAX_RF_PAYLOAD_SIZE);
             mTxBuf[0] = mid; // message id
             CP_U32_BigEndian(&mTxBuf[1], (invId  >> 8));
diff --git a/src/hm/payload.h b/src/hm/payload.h
index ef69c4f..8cdf6bd 100644
--- a/src/hm/payload.h
+++ b/src/hm/payload.h
@@ -77,9 +77,9 @@ class Payload : public Handler<payloadListenerType> {
 
         void add(packet_t *p, uint8_t len) {
             Inverter<> *iv = mSys->findInverter(&p->packet[1]);
-            if ((NULL != iv) && (p->packet[0] == (TX_REQ_INFO + ALL_FRAMES))) {  // response from get information command
+            if ((NULL != iv) && (p->packet[0] == (TX_REQ_DEVINFO + ALL_FRAMES))) {  // response from get information command
                 mPayload[iv->id].txId = p->packet[0];
-                DPRINTLN(DBG_DEBUG, F("Response from info request received"));
+                DPRINTLN(DBG_DEBUG, F("Response from DevInfo request received"));
                 uint8_t *pid = &p->packet[9];
                 if (*pid == 0x00) {
                     DPRINT(DBG_DEBUG, F("fragment number zero received and ignored"));
@@ -101,7 +101,7 @@ class Payload : public Handler<payloadListenerType> {
                 }
             }
             if ((NULL != iv) && (p->packet[0] == (TX_REQ_DEVCONTROL + ALL_FRAMES))) { // response from dev control command
-                DPRINTLN(DBG_DEBUG, F("Response from devcontrol request received"));
+                DPRINTLN(DBG_DEBUG, F("Response from DevControl request received"));
 
                 mPayload[iv->id].txId = p->packet[0];
                 iv->devControlRequest = false;
@@ -140,7 +140,7 @@ class Payload : public Handler<payloadListenerType> {
                 if (NULL == iv)
                     continue; // skip to next inverter
 
-                if ((mPayload[iv->id].txId != (TX_REQ_INFO + ALL_FRAMES)) && (0 != mPayload[iv->id].txId)) {
+                if ((mPayload[iv->id].txId != (TX_REQ_DEVINFO + ALL_FRAMES)) && (0 != mPayload[iv->id].txId)) {
                     // no processing needed if txId is not 0x95
                     // DPRINTLN(DBG_INFO, F("processPayload - set complete, txId: ") + String(mPayload[iv->id].txId, HEX));
                     mPayload[iv->id].complete = true;
@@ -160,7 +160,7 @@ class Payload : public Handler<payloadListenerType> {
                                         for (uint8_t i = 0; i < (mPayload[iv->id].maxPackId - 1); i++) {
                                             if (mPayload[iv->id].len[i] == 0) {
                                                 DPRINTLN(DBG_WARN, F("while retrieving data: Frame ") + String(i + 1) + F(" missing: Request Retransmit"));
-                                                mSys->Radio.sendCmdPacket(iv->radioId.u64, TX_REQ_INFO, (SINGLE_FRAME + i), true);
+                                                mSys->Radio.requestMainCmdPacket(iv->radioId.u64, TX_REQ_DEVINFO, (SINGLE_FRAME + i), true);
                                                 break;  // only retransmit one frame per loop
                                             }
                                             yield();
@@ -168,11 +168,11 @@ class Payload : public Handler<payloadListenerType> {
                                     } else {
                                         DPRINTLN(DBG_WARN, F("while retrieving data: last frame missing: Request Retransmit"));
                                         if (0x00 != mLastPacketId)
-                                            mSys->Radio.sendCmdPacket(iv->radioId.u64, TX_REQ_INFO, mLastPacketId, true);
+                                            mSys->Radio.requestMainCmdPacket(iv->radioId.u64, TX_REQ_DEVINFO, mLastPacketId, true);
                                         else {
                                             mPayload[iv->id].txCmd = iv->getQueuedCmd();
-                                            DPRINTLN(DBG_INFO, F("(#") + String(iv->id) + F(") sendTimePacket"));
-                                            mSys->Radio.sendTimePacket(iv->radioId.u64, mPayload[iv->id].txCmd, mPayload[iv->id].ts, iv->alarmMesIndex);
+                                            DPRINTLN(DBG_INFO, F("(#") + String(iv->id) + F(") requestDevInfoPacket"));
+                                            mSys->Radio.requestDevInfoPacket(iv->radioId.u64, mPayload[iv->id].txCmd, mPayload[iv->id].ts, iv->alarmMesIndex);
                                         }
                                     }
                                     mSys->Radio.switchRxCh(100);
@@ -206,7 +206,7 @@ class Payload : public Handler<payloadListenerType> {
                         if (NULL == rec) {
                             DPRINTLN(DBG_ERROR, F("record is NULL!"));
                         } else if ((rec->pyldLen == payloadLen) || (0 == rec->pyldLen)) {
-                            if (mPayload[iv->id].txId == (TX_REQ_INFO + 0x80))
+                            if (mPayload[iv->id].txId == (TX_REQ_DEVINFO + 0x80))
                                 stat->rxSuccess++;
 
                             rec->ts = mPayload[iv->id].ts;
diff --git a/src/web/web.h b/src/web/web.h
index d892274..ecb71b4 100644
--- a/src/web/web.h
+++ b/src/web/web.h
@@ -82,7 +82,7 @@ class Web {
 
             mWeb.on("/update",         HTTP_GET,  std::bind(&Web::onUpdate,       this, std::placeholders::_1));
             mWeb.on("/update",         HTTP_POST, std::bind(&Web::showUpdate,     this, std::placeholders::_1),
-                                                   std::bind(&Web::showUpdate2,    this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::place
holders::_4, std::placeholders::_5, std::placeholders::_6));
+                                                  std::bind(&Web::showUpdate2,    this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, std::placeholders::_5, std::placeholders::_6));
             mWeb.on("/serial",         HTTP_GET,  std::bind(&Web::onSerial,       this, std::placeholders::_1));
 
 
@@ -544,13 +544,13 @@ class Web {
             uint8_t cmd = response["cmd"];
             Inverter<> *iv = mSys->getInverterByPos(iv_id);
             if (NULL != iv) {
-                if (response["tx_request"] == (uint8_t)TX_REQ_INFO) {
+                if (response["tx_request"] == (uint8_t)TX_REQ_DEVINFO) {
                     // if the AlarmData is requested set the Alarm Index to the requested one
                     if (cmd == AlarmData || cmd == AlarmUpdate) {
                         // set the AlarmMesIndex for the request from user input
                         iv->alarmMesIndex = response["payload"];
                     }
-                    DPRINTLN(DBG_INFO, F("Will make tx-request 0x15 with subcmd ") + String(cmd) + F(" and payload ") + String((uint16_t) response["payload"]));
+                    DPRINTLN(DBG_INFO, F("Will make tx-request DevInfo 0x15 with subcmd ") + String(cmd) + F(" and payload ") + String((uint16_t) response["payload"]));
                     // process payload from web request corresponding to the cmd
                     iv->enqueCommand<InfoCommand>(cmd);
                 }
diff --git a/tools/rpi/hoymiles/__init__.py b/tools/rpi/hoymiles/__init__.py
index c02a0c7..667c4b7 100644
--- a/tools/rpi/hoymiles/__init__.py
+++ b/tools/rpi/hoymiles/__init__.py
@@ -483,7 +483,7 @@ def compose_send_time_payload(cmdId, alarm_id=0):
     """
     timestamp = int(time.time())
 
-    # indices from esp8266 hmRadio.h / sendTimePacket()
+    # indices from esp8266 hmRadio.h / requestDevInfoPacket()
     payload = struct.pack('>B', cmdId)                # 10
     payload = payload + b'\x00'                       # 11
     payload = payload + struct.pack('>L', timestamp)  # 12..15 big-endian: msb at low address

Speziell habe ich die Kommando / Methoden Namen refaktorisiert damit es für mich (TM) besser lesbar wird.
Mal sehen ob ich den u.a. Patch auch noch auf das aktuelle 0.5.68 development release angwendet bekomme ...
Cmd müsste man evtl. noch teilweise in SubCmd umbenennen um beim Hoymiles Lingo zu bleiben.

Hoffentlich bekomme ich irgendwann mal einen ordentlichen Workflow für Patches / PR mit PlatformIO/Git hin.

@stefan123t stefan123t added the enhancement New feature or request label Jan 12, 2023
@stefan123t stefan123t changed the title Hinweis zu app::loop Feature Request: Hinweis zu app::loop Optimierung Jan 12, 2023
@beegee3 beegee3 mentioned this issue Feb 5, 2023
@lumapu lumapu added the fixed dev fixed label Feb 25, 2023
@beegee3 beegee3 closed this as completed Feb 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed dev fixed
Projects
None yet
Development

No branches or pull requests

3 participants