From 4f17ed4b5dfdb6eb3841455216a8007aeb1320c2 Mon Sep 17 00:00:00 2001 From: mumpf Date: Tue, 1 Mar 2022 17:28:11 +0100 Subject: [PATCH 1/3] Feature: Uninitialized GO handling (#187) - don't send senseless values on GroupValueRead Co-authored-by: Waldemar Porscha --- src/knx/group_object.cpp | 7 +++++++ src/knx/group_object.h | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/knx/group_object.cpp b/src/knx/group_object.cpp index db92058..6f19247 100644 --- a/src/knx/group_object.cpp +++ b/src/knx/group_object.cpp @@ -74,6 +74,10 @@ bool GroupObject::readEnable() if (!_table) return false; + // we forbid reading of new (uninitialized) go + if (_commFlag == Uninitialized) + return false; + return bitRead(ntohs(_table->_tableData[_asap]), 11) > 0; } @@ -270,5 +274,8 @@ void GroupObject::valueNoSend(const KNXValue& value) void GroupObject::valueNoSend(const KNXValue& value, const Dpt& type) { + if (_commFlag == Uninitialized) + _commFlag = Ok; + KNX_Encode_Value(value, _data, _dataLength, type); } diff --git a/src/knx/group_object.h b/src/knx/group_object.h index 91c8b56..88ce274 100644 --- a/src/knx/group_object.h +++ b/src/knx/group_object.h @@ -14,7 +14,8 @@ enum ComFlag WriteRequest = 2, //!< Write was requested but was not processed Transmitting = 3, //!< Group Object is processed a the moment (read or write) Ok = 4, //!< read or write request were send successfully - Error = 5 //!< there was an error on processing a request + Error = 5, //!< there was an error on processing a request + Uninitialized = 6 //!< uninitialized Group Object, its value is not valid }; class GroupObject; @@ -235,7 +236,7 @@ class GroupObject size_t asapValueSize(uint8_t code); size_t goSize(); uint16_t _asap = 0; - ComFlag _commFlag = Ok; + ComFlag _commFlag = Uninitialized; uint8_t* _data = 0; uint8_t _dataLength = 0; #ifndef SMALL_GROUPOBJECT From 145c3541105ea57d1f9b2306ba49c6e4b6361d75 Mon Sep 17 00:00:00 2001 From: mptei Date: Tue, 1 Mar 2022 17:30:54 +0100 Subject: [PATCH 2/3] Removed unneded TX_WAIT_ECHO state; fixed echo detection (#188) --- src/knx/tpuart_data_link_layer.cpp | 56 ++++++------------------------ 1 file changed, 11 insertions(+), 45 deletions(-) diff --git a/src/knx/tpuart_data_link_layer.cpp b/src/knx/tpuart_data_link_layer.cpp index 2b89c9a..c32d460 100644 --- a/src/knx/tpuart_data_link_layer.cpp +++ b/src/knx/tpuart_data_link_layer.cpp @@ -85,7 +85,6 @@ enum { TX_IDLE, TX_FRAME, - TX_WAIT_ECHO, TX_WAIT_CONN }; @@ -138,7 +137,6 @@ void TpUartDataLinkLayer::loop() // Loop once and repeat as long we have rx data available do { // Signals to communicate from rx part with the tx part - bool isEchoComplete = false; // Flag that a complete echo is received uint8_t dataConnMsg = 0; // The DATA_CONN message just seen or 0 #ifdef KNX_WAIT_FOR_ADDR @@ -296,22 +294,14 @@ void TpUartDataLinkLayer::loop() if (_RxByteCnt == 7) { //Destination Address + payload available - //check if echo - if (_sendBuffer != nullptr && (!((buffer[0] ^ _sendBuffer[0]) & ~0x20) && !memcmp(buffer + _convert + 1, _sendBuffer + 1, 5))) - { //ignore repeated bit of control byte - _isEcho = true; - } - else - { - _isEcho = false; - } + //check if echo; ignore repeat bit of control byte + _isEcho = (_sendBuffer != nullptr && (!((buffer[0] ^ _sendBuffer[0]) & ~0x20) && !memcmp(buffer + _convert + 1, _sendBuffer + 1, 5))); //convert into Extended.ind if (_convert) { - uint8_t payloadLength = buffer[6] & 0x0F; buffer[1] = buffer[6] & 0xF0; - buffer[6] = payloadLength; + buffer[6] &= 0x0F; } if (!_isEcho) @@ -358,22 +348,10 @@ void TpUartDataLinkLayer::loop() if (_RxByteCnt == buffer[6] + 7 + 2) { //complete Frame received, payloadLength+1 for TCPI +1 for CRC + //check if crc is correct if (rxByte == (uint8_t)(~_xorSum)) { - //check if crc is correct - if (_isEcho && _sendBuffer != NULL) - { - //check if it is realy an echo, rx_crc = tx_crc - if (rxByte == _sendBuffer[_sendBufferLength - 1]) - _isEcho = true; - else - _isEcho = false; - } - if (_isEcho) - { - isEchoComplete = true; - } - else + if (!_isEcho) { _receiveBuffer[0] = 0x29; _receiveBuffer[1] = 0; @@ -428,8 +406,8 @@ void TpUartDataLinkLayer::loop() } while (_rxState == RX_L_ADDR && (stayInRx || _platform.uartAvailable())); // Check for spurios DATA_CONN message - if (dataConnMsg && _txState != TX_WAIT_CONN && _txState != TX_WAIT_ECHO) { - println("got unexpected L_DATA_CON"); + if (dataConnMsg && _txState != TX_WAIT_CONN) { + println("unexpected L_DATA_CON"); } switch (_txState) @@ -450,9 +428,9 @@ void TpUartDataLinkLayer::loop() if (sendSingleFrameByte() == false) { _waitConfirmStartTime = millis(); - _txState = TX_WAIT_ECHO; + _txState = TX_WAIT_CONN; #ifdef DBG_TRACE - println("TX_WAIT_ECHO"); + println("TX_WAIT_CONN"); #endif } else @@ -461,22 +439,10 @@ void TpUartDataLinkLayer::loop() } } break; - case TX_WAIT_ECHO: case TX_WAIT_CONN: - if (isEchoComplete) + if (dataConnMsg) { - _txState = TX_WAIT_CONN; -#ifdef DBG_TRACE - println("TX_WAIT_CONN"); -#endif - } - else if (dataConnMsg) - { - bool waitEcho = (_txState == TX_WAIT_ECHO); - if (waitEcho) { - println("L_DATA_CON without echo"); - } - dataConBytesReceived(_receiveBuffer, _RxByteCnt + 2, !waitEcho && ((dataConnMsg & SUCCESS) > 0)); + dataConBytesReceived(_receiveBuffer, _RxByteCnt + 2, (dataConnMsg & SUCCESS)); delete[] _sendBuffer; _sendBuffer = 0; _sendBufferLength = 0; From a10fa20bde3b70413a74af5e750ba18801520029 Mon Sep 17 00:00:00 2001 From: mumpf Date: Tue, 1 Mar 2022 21:42:03 +0100 Subject: [PATCH 3/3] Feature: additional callbacks for application (#189) * Feature: additional callbacks for application - beforeRestart callback - beforeTablesUnload callback * naming conventions for PR Co-authored-by: Waldemar Porscha --- src/knx/address_table_object.cpp | 4 +++- src/knx/association_table_object.cpp | 1 + src/knx/bau.cpp | 9 +++++++++ src/knx/bau.h | 4 ++++ src/knx/bau_systemB.cpp | 12 ++++++++++++ src/knx/bau_systemB.h | 3 +++ src/knx/group_object_table_object.cpp | 1 + src/knx/table_object.cpp | 26 ++++++++++++++++++++++++++ src/knx/table_object.h | 18 ++++++++++++++---- src/knx_facade.h | 13 ++++++++++++- 10 files changed, 85 insertions(+), 6 deletions(-) diff --git a/src/knx/address_table_object.cpp b/src/knx/address_table_object.cpp index 689580c..c2e9154 100644 --- a/src/knx/address_table_object.cpp +++ b/src/knx/address_table_object.cpp @@ -19,7 +19,8 @@ AddressTableObject::AddressTableObject(Memory& memory) uint16_t AddressTableObject::entryCount() { - if (loadState() != LS_LOADED) + // after programming without GA the module hangs + if (loadState() != LS_LOADED || _groupAddresses[0] == 0xFFFF) return 0; return ntohs(_groupAddresses[0]); @@ -67,6 +68,7 @@ bool AddressTableObject::contains(uint16_t addr) void AddressTableObject::beforeStateChange(LoadState& newState) { + TableObject::beforeStateChange(newState); if (newState != LS_LOADED) return; diff --git a/src/knx/association_table_object.cpp b/src/knx/association_table_object.cpp index 0880fc7..92cc839 100644 --- a/src/knx/association_table_object.cpp +++ b/src/knx/association_table_object.cpp @@ -60,6 +60,7 @@ int32_t AssociationTableObject::translateAsap(uint16_t asap) void AssociationTableObject::beforeStateChange(LoadState& newState) { + TableObject::beforeStateChange(newState); if (newState != LS_LOADED) return; diff --git a/src/knx/bau.cpp b/src/knx/bau.cpp index a74e7bf..24f8138 100644 --- a/src/knx/bau.cpp +++ b/src/knx/bau.cpp @@ -338,3 +338,12 @@ void BusAccessUnit::propertyValueWrite(ObjectType objectType, uint8_t objectInst uint8_t* data, uint32_t length) { } + +void BusAccessUnit::beforeRestartCallback(BeforeRestartCallback func) +{ +} + +BeforeRestartCallback BusAccessUnit::beforeRestartCallback() +{ + return 0; +} diff --git a/src/knx/bau.h b/src/knx/bau.h index 78b3580..1382446 100644 --- a/src/knx/bau.h +++ b/src/knx/bau.h @@ -3,6 +3,8 @@ #include "knx_types.h" #include "interface_object.h" +typedef void (*BeforeRestartCallback)(void); + class BusAccessUnit { public: @@ -161,4 +163,6 @@ class BusAccessUnit virtual void propertyValueWrite(ObjectType objectType, uint8_t objectInstance, uint8_t propertyId, uint8_t& numberOfElements, uint16_t startIndex, uint8_t* data, uint32_t length); + virtual void beforeRestartCallback(BeforeRestartCallback func); + virtual BeforeRestartCallback beforeRestartCallback(); }; diff --git a/src/knx/bau_systemB.cpp b/src/knx/bau_systemB.cpp index 9c6fb0a..11d4d35 100644 --- a/src/knx/bau_systemB.cpp +++ b/src/knx/bau_systemB.cpp @@ -152,6 +152,8 @@ void BauSystemB::restartRequestIndication(Priority priority, HopCountType hopTyp if (restartType == RestartType::BasicRestart) { println("Basic restart requested"); + if (_beforeRestart != 0) + _beforeRestart(); } else if (restartType == RestartType::MasterReset) { @@ -611,3 +613,13 @@ Memory& BauSystemB::memory() { return _memory; } + +void BauSystemB::beforeRestartCallback(BeforeRestartCallback func) +{ + _beforeRestart = func; +} + +BeforeRestartCallback BauSystemB::beforeRestartCallback() +{ + return _beforeRestart; +} diff --git a/src/knx/bau_systemB.h b/src/knx/bau_systemB.h index abbc653..9ba9d65 100644 --- a/src/knx/bau_systemB.h +++ b/src/knx/bau_systemB.h @@ -38,6 +38,8 @@ class BauSystemB : protected BusAccessUnit void propertyValueWrite(ObjectType objectType, uint8_t objectInstance, uint8_t propertyId, uint8_t& numberOfElements, uint16_t startIndex, uint8_t* data, uint32_t length) override; + void beforeRestartCallback(BeforeRestartCallback func); + BeforeRestartCallback beforeRestartCallback(); protected: virtual ApplicationLayer& applicationLayer() = 0; @@ -107,4 +109,5 @@ class BauSystemB : protected BusAccessUnit RestartState _restartState = Idle; SecurityControl _restartSecurity; uint32_t _restartDelay = 0; + BeforeRestartCallback _beforeRestart = 0; }; diff --git a/src/knx/group_object_table_object.cpp b/src/knx/group_object_table_object.cpp index b610404..bdcf8dd 100644 --- a/src/knx/group_object_table_object.cpp +++ b/src/knx/group_object_table_object.cpp @@ -77,6 +77,7 @@ void GroupObjectTableObject::groupObjects(GroupObject * objs, uint16_t size) void GroupObjectTableObject::beforeStateChange(LoadState& newState) { + TableObject::beforeStateChange(newState); if (newState != LS_LOADED) return; diff --git a/src/knx/table_object.cpp b/src/knx/table_object.cpp index 44c504e..1f670a9 100644 --- a/src/knx/table_object.cpp +++ b/src/knx/table_object.cpp @@ -6,6 +6,19 @@ #include "callback_property.h" #include "data_property.h" +BeforeTablesUnloadCallback TableObject::_beforeTablesUnload = 0; +uint8_t TableObject::_tableUnloadCount = 0; + +void TableObject::beforeTablesUnloadCallback(BeforeTablesUnloadCallback func) +{ + _beforeTablesUnload = func; +} + +BeforeTablesUnloadCallback TableObject::beforeTablesUnloadCallback() +{ + return _beforeTablesUnload; +} + TableObject::TableObject(Memory& memory) : _memory(memory) {} @@ -13,6 +26,19 @@ TableObject::TableObject(Memory& memory) TableObject::~TableObject() {} +void TableObject::beforeStateChange(LoadState& newState) +{ + if (newState == LS_LOADED && _tableUnloadCount > 0) + _tableUnloadCount--; + if (_tableUnloadCount > 0) + return; + if (newState == LS_UNLOADED) { + _tableUnloadCount++; + if (_beforeTablesUnload != 0) + _beforeTablesUnload(); + } +} + LoadState TableObject::loadState() { return _state; diff --git a/src/knx/table_object.h b/src/knx/table_object.h index c4213b3..66768cd 100644 --- a/src/knx/table_object.h +++ b/src/knx/table_object.h @@ -3,6 +3,9 @@ #include "interface_object.h" class Memory; + +typedef void (*BeforeTablesUnloadCallback)(); + /** * This class provides common functionality for interface objects that are configured by ETS with MemorWrite. */ @@ -28,14 +31,18 @@ class TableObject: public InterfaceObject uint8_t* save(uint8_t* buffer) override; const uint8_t* restore(const uint8_t* buffer) override; uint16_t saveSize() override; - protected: + + static void beforeTablesUnloadCallback(BeforeTablesUnloadCallback func); + static BeforeTablesUnloadCallback beforeTablesUnloadCallback(); + + protected: /** * This method is called before the interface object enters a new ::LoadState. * If there is a error changing the state newState should be set to ::LS_ERROR and errorCode() * to a reason for the failure. */ - virtual void beforeStateChange(LoadState& newState) {} - + virtual void beforeStateChange(LoadState& newState); + /** * returns the internal data of the interface object. This pointer belongs to the TableObject class and * must not be freed. @@ -47,7 +54,9 @@ class TableObject: public InterfaceObject void errorCode(ErrorCode errorCode); void initializeProperties(size_t propertiesSize, Property** properties) override; - + + static BeforeTablesUnloadCallback _beforeTablesUnload; + private: uint32_t tableReference(); bool allocTable(uint32_t size, bool doFill, uint8_t fillByte); @@ -68,6 +77,7 @@ class TableObject: public InterfaceObject LoadState _state = LS_UNLOADED; Memory& _memory; uint8_t *_data = 0; + static uint8_t _tableUnloadCount; /** * used to store size of data() in allocTable(), needed for calculation of crc in PID_MCB_TABLE. diff --git a/src/knx_facade.h b/src/knx_facade.h index ecfbda0..dbd303a 100644 --- a/src/knx_facade.h +++ b/src/knx_facade.h @@ -400,7 +400,18 @@ template class KnxFacade : private SaveRestore void restart(uint16_t individualAddress) { - _bau.restartRequest(individualAddress); + SecurityControl sc = {false, None}; + _bau.restartRequest(individualAddress, sc); + } + + void beforeRestartCallback(BeforeRestartCallback func) + { + _bau.beforeRestartCallback(func); + } + + BeforeRestartCallback beforeRestartCallback() + { + return _bau.beforeRestartCallback(); } private: