thelsing / knx

knx stack (TP, IP and RF) for arduino and linux, Can be configured with ETS
GNU General Public License v3.0
259 stars 91 forks source link

Implement way to generate knx-serial number from platform #90

Closed mjm987 closed 3 years ago

mjm987 commented 3 years ago

When more than one KNX-RF device is active, nodes where knx.progMode() is off are unexpectedly programmed as well when programming the physical address of a new node. So it seems the state of knx.progMode() has no effect.

Shouldn't prgMode() checked in following methods of bau27B0.cpp: void Bau27B0::domainAddressSerialNumberWriteIndication() void Bau27B0::individualAddressSerialNumberWriteIndication()

(Btw: to handle progMode without a button I tried activating knx.ProgMode(true) in the setup() of .ino and disable it again in the .ino loop() after knx.configured() && at least 1 Min passed since reset)

nanosonde commented 3 years ago

@mjm987 Hi! You seem to be the first one who actually tries this. :) While implementing KNX-RF I have never tested this to be honest.

Do you have multiple DIY KNX-RF devices? If so, did you set a new serial number for each device so that they differ? All application layer services which have something with "SerialNumber" in their name do NOT take care about the prog button. Compare https://github.com/thelsing/knx/blob/master/src/knx/bau_systemB.cpp#L447 and https://github.com/thelsing/knx/blob/master/src/knx/bau_systemB.cpp#L441

The serial number is unique for every KNX device of a KNX manufacturer.

nanosonde commented 3 years ago

BTW: The KNX serial number consists of 4 bytes. So a KNX manufacturer can sell 4294967295 (0xFFFFFFFF) devices. Ok, 4294967295 - 2 devices. 0xFFFFFFFF and 0x00000000 are not valid. :)

mjm987 commented 3 years ago

Hi @nanosonde, thx for response! Yes I'm trying with a couple of nodes to build a garden sprinkler with valves and humidity sensors connected by KNX-RF. And no, I didn't change the serial number and let it to "Default to KNXA (0xFA)". According to the referenced comment in the source I expected it is ok for ETS to have always the same serial so I will possible try to generate a random on initial start or first lookup of the serial. (I didn't like the idea to make a individual program change for every new board.) To handle the problem temporarily, I hardcoded ownAddress on newly devices and prevented to program the physical address by ETS.

nanosonde commented 3 years ago

And no, I didn't change the serial number and let it to "Default to KNXA (0xFA)".

The manufacturer part must be kept as 0xFA. Otherwise the DIY generated product databases won‘t work when ETS reads the manufacturer.

According to the referenced comment in the source I expected it is ok for ETS to have always the same serial so I will possible try to generate a random on initial start or first lookup of the serial. A random serialnumber is not good if it changes on every startup. Random once and then store it or use a factory serial number of the MCU to generate the KNX serial number.

If you want to use KNX Data Secure later a unique KNX serial number is a MUST as ETS uses the KNX serial number as an index to store the associated encryption keys.

(I didn't like the idea to make a individual program change for every new board.)

Maybe we should introduce some generic mechanism that generates a random KNX serial number once after flashing and store it. However, after reflashing the board, a new KNX serialnumber would be really bad if the device is already known to ETS.

One Approach could be a simple patcher that patches a well-known location of a binary to change the KNX serial number. It could also read it.

mjm987 commented 3 years ago

Later using KNX Data Secure is a good point! In the SAMD21 Datasheet I saw that: "Each device has a unique 128-bit serial number which is a concatenation of four 32-bit words contained at thefollowing addresses: Word 0: 0x0080A00C Word 1: 0x0080A040 Word 2: 0x0080A044 Word 3: 0x0080A048 The uniqueness of the serial number is guaranteed only when using all 128 bits. "

So I think we could easily use it but to get a (hopefully) unique id we need to calculate a simple hash (XOR?), because the KNX Serial is only 6 bytes long (?)

(On the CC1101 I didn't found a serial.)

mjm987 commented 3 years ago

BTW: having individual serialNumber in device_object.cpp seems not to solve the problem because the Serial Number in knxprod has no influence (otherwise a new knxprod would be needed for every new instance of the same product). So I guess programming Domain Address (in "rf_medium_object") and ownAddress (in "device_object") only if progMode() is true seems to be needed.

mjm987 commented 3 years ago

Following patch solves the programming with multiple RF devices for me:

diff --git a/src/knx/bau27B0.cpp b/src/knx/bau27B0.cpp
index c9cf495..b4c6bb8 100644
--- a/src/knx/bau27B0.cpp
+++ b/src/knx/bau27B0.cpp
@@ -121,8 +121,10 @@ void Bau27B0::domainAddressSerialNumberWriteIndication(Priority priority, HopCou
 {
     // If the received serial number matches our serial number
     // then store the received RF domain address in the RF medium object
-    if (!memcmp(knxSerialNumber, _deviceObj.propertyData(PID_SERIAL_NUMBER), 6))
-        _rfMediumObj.rfDomainAddress(rfDoA);
+    if (_deviceObj.progMode()) {
+        if (!memcmp(knxSerialNumber, _deviceObj.propertyData(PID_SERIAL_NUMBER), 6))
+            _rfMediumObj.rfDomainAddress(rfDoA);
+    }
 }

 void Bau27B0::domainAddressSerialNumberReadIndication(Priority priority, HopCountType hopType, const uint8_t* knxSerialNumber)
@@ -138,6 +140,8 @@ void Bau27B0::individualAddressSerialNumberWriteIndication(Priority priority, Ho
 {
     // If the received serial number matches our serial number
     // then store the received new individual address in the device object
-    if (!memcmp(knxSerialNumber, _deviceObj.propertyData(PID_SERIAL_NUMBER), 6))
-        _deviceObj.induvidualAddress(newIndividualAddress);
+    if (_deviceObj.progMode()) {
+        if (!memcmp(knxSerialNumber, _deviceObj.propertyData(PID_SERIAL_NUMBER), 6))
+            _deviceObj.induvidualAddress(newIndividualAddress);
+    }
 }

By the way: on a Raspberry Pi where I tested the KNX-RF problem, toggling progMode with the button didn't work (I tried with different pins and of course uncommenting knx.buttonPin()). I soved this without button by enabling progMode at start of the program (even when configured) and deactivating it on minute later in case the device is configured by:

diff --git a/examples/knx-linux/main.cpp b/examples/knx-linux/main.cpp
index 1345847..f16574b 100644
--- a/examples/knx-linux/main.cpp
+++ b/examples/knx-linux/main.cpp
@@ -15,6 +15,8 @@
 #include <sched.h>
 #include <sys/mman.h>

+#define PROGRAM_MODE_TIMEOUT (60UL * 1000UL)  // 1 min after Reset
+
 volatile sig_atomic_t loopActive = 1;
 void signalHandler(int sig)
 {
@@ -41,6 +43,7 @@ KnxFacade<LinuxPlatform, Bau27B0> knx;
 #endif

 long lastsend = 0;
+unsigned long program_mode_timeout;

 #define CURR knx.getGroupObject(1)
 #define MAX knx.getGroupObject(2)
@@ -91,9 +94,12 @@ void appLoop()
 void setup()
 {
 //knx.buttonPin(5);
+    program_mode_timeout = millis() + PROGRAM_MODE_TIMEOUT;
     srand((unsigned int)time(NULL));
     knx.readMemory();

+    printf("knx.individualAddress = %x\n", knx.induvidualAddress());
+
     if (knx.induvidualAddress() == 0)
         knx.progMode(true);

@@ -115,6 +121,10 @@ void setup()
     else
         println("not configured");
     knx.start();
+
+knx.progMode(true);
+   printf("progMode: %s\n", knx.progMode()?"true":"false");
+
 }

 int main(int argc, char **argv)
@@ -135,12 +145,16 @@ int main(int argc, char **argv)
     knx.platform().cmdLineArgs(argc, argv);

     setup();
-    
     while (loopActive)
     {
         knx.loop();
         if(knx.configured())
             appLoop();
+        else if (knx.progMode() && millis() > program_mode_timeout) {
+        println("auto disable ProgMode");
+         knx.progMode(false);
+    }
+
         delayMicroseconds(100);
     }
nanosonde commented 3 years ago

I have analyzed the problem further.

I think the issue results from the fact that the stack does NOT implement the following application layer services:

For RF I had only implemented:

According to the specs, the application layer services that use the KNX serial number do NOT check the programming mode. These are:

The counterparts that are referring to the programming mode are:

Compare those: https://github.com/calimero-project/calimero-device/blob/606850ce4566bf20a4f49e6bb40a575f30cd6895/src/main/java/tuwien/auto/calimero/device/KnxDeviceServiceLogic.java#L607 https://github.com/calimero-project/calimero-device/blob/606850ce4566bf20a4f49e6bb40a575f30cd6895/src/main/java/tuwien/auto/calimero/device/KnxDeviceServiceLogic.java#L614 https://github.com/calimero-project/calimero-device/blob/606850ce4566bf20a4f49e6bb40a575f30cd6895/src/main/java/tuwien/auto/calimero/device/KnxDeviceServiceLogic.java#L633 https://github.com/calimero-project/calimero-device/blob/606850ce4566bf20a4f49e6bb40a575f30cd6895/src/main/java/tuwien/auto/calimero/device/KnxDeviceServiceLogic.java#L687

Do you see any error message in the logs about unknown APDU types with any of the following values? https://github.com/thelsing/knx/blob/aabc61dc3b33116e0daabb6c22f78de8b8d205da/src/knx/knx_types.h#L152-L154

nanosonde commented 3 years ago

BTW: having individual serialNumber in device_object.cpp seems not to solve the problem because the Serial Number in knxprod has no influence (otherwise a new knxprod would be needed for every new instance of the same product).

I disagree. :slightly_smiling_face: It IS important to have a proper serial number in device_object.cpp. And yes, it is NOT related to a knxprod file. You are correct: it would a new file for each and every device what is not feasable.

I have two certified KNX-RF devices here: MDT TP/RF coupler and MDT roller-shutter actuator. ETS version is v5.7.4.

ETS has read out both devices and has stored the serial number in its database. I did not enter any serial number by myself.

grafik

The serial number is from the device object:

grafik

I have just verified what happens when I try to program the individual address to the KNX-RF device (actuator). ETS sends "SystemNetworkParameterRead" messges with parameter NM_Read_SerialNumber_By_ProgrammingMode from Device Object and PID_SERIAL_NUMBER. See here.

So ETS wants to read the serial numbers of ALL devices where currently in programming mode. Only devices in programming mode send a reply with their serial number. So ETS has the serial number. It then uses this serial number to address a specific device by its serial number and using any of:

As a result: having exactly the same serial number for multiple devices will lead to problems.

mjm987 commented 3 years ago

Thx for your brilliant explanation and understanding!

1.) When specifying a individual serialNumber in the device, the serial is shown indeed in ETS but in the form < 2 Bytes Manufacturer ID > : < 4 Bytes of serialNumber > This is because the first 2 bytes of the 6 byte serialNumber is overwritten in device_object.cpp So the hardware specific part of the serialNumber seems to be only 4 bytes right?

2.) Yes I agree with your explanation above, but the problem is that the when programming the physical address, not only the device which is in programming mode seems to response but also other thelsing-knx devices which are not in programming mode. Otherwise with the two small changes in bau27B0.cpp as described here the probem has gone (even when the devices have the same serialNumber.

3.)

nanosonde commented 3 years ago

1) Within the actual KNX packets the KNX serial number is always 6 bytes. As you said: 2 bytes manufacturer and 4 bytes actual serial number. As the first two bytes are fixed to 00FA, only the last 4 bytes may vary. Data secure also forbids 0x00000000 for the 4 bytes. My guess is that 0xFFFFFFFF is also not valid. The manufacturer id 00FA must NOT be modified as the self-generated knxprod files won't work. They are signed as manufacturer 00FA.

2) Could you please elaborate a bit more what exactly you see? Do you have the possibility to use Net'N'Node from Weinzierl? There is a free version after registration.

Have a look here: https://github.com/thelsing/knx/blob/aabc61dc3b33116e0daabb6c22f78de8b8d205da/src/knx/bau_systemB.cpp#L539-L546

As you can see the KNX stack should only send the serial nummer in the response if programming mode is active.

It would be cool if you could trace the KNX traffic using the mentioned tool if multiple device are answering although they should not.

As programming a physical address involves various application layer service messages (APCI types), we have to find out where exactly the problem is.

nanosonde commented 3 years ago

Forgot one question: You wrote that other thelsing-knx devices which are not in programming mode would answer. What KNX serial number does each of it have? Are they purely KNX-RF?

To use multiple thelsing-knx RF-devices you MUST compile a new firmware image for EACH of it as for KNX open medium protocols things work differently as for normal TP1 devices. Each firmware image MUST have its own dedicated KNX serial number, i.e. the last 4 bytes MUST differ in the device object.

Current status is: if you do NOT do it then if ETS tries to talk to a device whose KNX serial number is NOT unique then domainAddressSerialNumberReadIndication for example would be answered by all other KNX RF devices which have the same KNX serial number. This is NOT protected by having programming mode disabled. As a result: If you know the KNX serial number you can always write to the device independent of the programming mode status.

mjm987 commented 3 years ago

Yes, the behaviour was with same serialNumber on the devices. I could do the tests but it will take some time to revert my changes and implement individual serialNumbers. (I'm currently experimenting with CC1310 Modules (EBYTE E70 from Aliexpress) but my port is still experimental and unclean)

nanosonde commented 3 years ago

ok, take your time. What exactly are doing within ETS? Programming physical address only? Partial download? Application download? Physical address + application programming?

I am asking because if you only try to program the physcial address, ETS should first start the procedure mentioned above: sending SystemNetworkParameterRead to get the serial number which needs programming. However, it only sends a response if the device is in programming mode.

So if you say that multiple devices are answering what kind of messages are they?

mjm987 commented 3 years ago

Ok, I tested now with individual serialNumber and you are perfectly right: other devices which are not in program mode don't respond now and are not overwritten. Also Net'n Node shows no errors.

For testing purposes, adding individual serials for SAMD21 platform I have done as follows in device_object.cpp: `

define SAMD21_SERIAL0 (volatile uint32_t)(0x0080A00C)

define SAMD21_SERIAL1 (volatile uint32_t)(0x0080A040)

define SAMD21_SERIAL2 (volatile uint32_t)(0x0080A044)

define SAMD21_SERIAL3 (volatile uint32_t)(0x0080A048)

(uint32_t)(serialNumber+2) = SAMD21_SERIAL0 ^ SAMD21_SERIAL1 ^ SAMD21_SERIAL2 ^ SAMD21_SERIAL3; ` (As mentioned in a previous post the SAMD21 has a 4 x 32bit Serial (on non-contiguous addresses!) which I xor above to get a individual 32bit serial even if part of the 128bit devices serial is same on two devices. Further i save this 32bit starting at offset 2 of serialNumber because [0] and [1] is overwritten by the manufacturer Id.)

Because estimating the serial is platform dependant, getting the device serial should be done in samd_platform.cpp (or as macro to samd_platform.h) and then referenced in device_object.cpp. Am I right?

thelsing commented 3 years ago

Yes. Serial number generation from some hardware-serial should be added to the Platform class.

nanosonde commented 3 years ago

Oops. I forgot to add the serial number stuff to the CC1310 platform. If we add it there, how do we handle the other KNX-RF solution based on CC1101? I do not have an overview, but is it possible to read a serial number from each MCU of the currently supported platforms (SAMD, ESP*, STM32, Linux)?

thelsing commented 3 years ago

I think some kind of serial number of the MCU, Flash etc is nearly always available. The question is: Is the way to determine a serial number platform specific or application specific?

mjm987 commented 3 years ago

I would recommend platform specific. SAMD21 you see above and for ESP* and Linux you could take a simple XOR over the Ethernet or WiFi Mac address. I think STM32 has no factory serial but you could eg. take the millis() between reset and the first push of the Program Button and save this in the Flash. (But you would loose it when reprogramming the firmware but I guess this shouldn't be a problem.)

mjm987 commented 3 years ago

At least some STM32 have a HW random generator.

etrinh commented 3 years ago

https://github.com/ricaun/ArduinoUniqueID may it help?

mjm987 commented 3 years ago

Indeed https://github.com/ricaun/ArduinoUniqueID/blob/master/src/ArduinoUniqueID.cpp is very nice. In case the UniqueID of a device as more than 32 bits, you should eg. xor every 32-bit (because you only need 32 bits and don't know which part of the UniqueID is fix and which changes in a production series).

OutOfSync1 commented 3 years ago

I have added UID generation as suggested to the platform class (https://github.com/OutOfSync1/knx/commit/4018d98d0e5e65d536bc14ef5239fb3827058862). This should work for ESP8226/ESP32 and the STM32/SAMD platforms. All others will use the default serial 0x01020304. I only tested with ESP32.

The serial is set right after the manufacturerId is forced to 0xFA in KnxFacade(): https://github.com/OutOfSync1/knx/blob/4018d98d0e5e65d536bc14ef5239fb3827058862/src/knx_facade.h#L54-L74

thelsing commented 3 years ago

Please submit a pull request.

Phil1pp commented 2 years ago

grafik

What tool is that?

nanosonde commented 2 years ago

What tool is that?

German: https://www.weinzierl.de/index.php/de/alles-knx1/software-tools/net-n-node English: https://www.weinzierl.de/index.php/en/all-knx/software-tools-en/net-n-node-en

Just get a free license by writing an email to them as described here: https://www.weinzierl.de/index.php/en/all-knx/software-tools-en/net-n-node-en-reg