Hello Martin,

Thanks for working on ddns, but *please* read Peter’s email.

This is now the third patch (I don’t know why you are at number 6 already) and the idea of the review process is to submit something that at least works. You can ask all sorts of questions about anything you need to know, but you will have to listen to what people answer.

This patch is fundamentally broken. I will comment on that further below.

Secondly, we cannot even think about merging something that isn’t tested at all.

So I would urge you to look at any of the other providers, because what you are trying to implement has already been implemented here for example:

  https://git.ipfire.org/?p=ddns.git;a=blob;f=src/ddns/providers.py;h=56e6620c78ab3d9de1c945147f86e70a8d8614d7;hb=HEAD#l975

@Stefan: We might want to have a class variable in the DynDNS protocol that adds the IPv6 address to a field that is named in that variable. That way, we avoid any code duplication.

On 18 May 2021, at 19:49, Martin Krieger <makrie@posteo.de> wrote:

From 37fb0237932fb12bdd635e9cb5e01b0bf9f03dda Mon Sep 17 00:00:00 2001
From: Martin Krieger <makrie@posteo.de>
Date: Tue, 18 May 2021 19:42:44 +0200
Subject: [PATCH v6 1/1] ddns: Add provider Feste-IP.Net

Comments:
Provider supports IPv4, IPv6 & DS (Dual-Stack)

Changelog:

18.05.2021
Improved failure handling.
IPv6 & DS still not checked, because my network connection is pure IPv4.

17.05.2021
5th attempt ([PATCH v5 1/1])

16.05.2021
4th attempt ([PATCH v4 1/1])

08.05.2021
3rd attempt ([PATCH v3 1/1])

06.05.2021
2nd attempt ([PATCH v2 1/1])

05.05.2021
1st attempt ([PATCH v1 1/1])

Signed-off-by: Martin Krieger <makrie@posteo.de>..
README                |  1 +
src/ddns/providers.py | 31 +++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+)

diff --git a/README b/README
index b6decb35c338..fa6ce5e598b8 100644
--- a/README
+++ b/README
@@ -68,6 +68,7 @@ SUPPORTED PROVIDERS:
	easydns.com
	enom.com
	entrydns.net
+	feste-ip.net
	freedns.afraid.org
	inwx.com|de|at|ch|es
	itsdns.de
diff --git a/src/ddns/providers.py b/src/ddns/providers.py
index 56e6620c78ab..b7e14482a0b1 100644
--- a/src/ddns/providers.py
+++ b/src/ddns/providers.py
@@ -1179,6 +1179,37 @@ class DDNSProviderEntryDNS(DDNSProvider):
		raise DDNSUpdateError


+class DDNSProviderFesteIPNet(DDNSProtocolDynDNS2, DDNSProvider):
+        handle    = "feste-ip.net"
+        name      = "Feste-IP.Net"
+        website   = "https://www.feste-ip.net/"
+
+        # Information about the format of the request is to be found
+        # https://forum.feste-ip.net/viewtopic.php?f=13&t=469
+
+        myips = ("myip","myip2")
+
+        url = "https://members.feste-ip.net/nic/update/"
+
+        def update(self):
+                data = {
+                        "hostname" : self.hostname
+                }
+
+                for proto in DDNSProvider.protocols:
+                        idx = 0
+                        tmpip = self.get_address(proto)
+                        if tmpip:
+                                data[self.myips[idx]] = tmpip
+                                idx += 1

Setting idx is useless here. You will reset to zero in every iteration of the loop. Since it is being incremented last and then reset to zero, idx will *always* be zero.

+
+                if self.myips[0] in data:
+                        self.send_request(data)
+                        return#

You will only send one request here and only if it contains an IPv4 address. Why? What happens to systems that only have IPv6?

-Michael

+
+                raise DDNSUpdateError
+
+
class DDNSProviderFreeDNSAfraidOrg(DDNSProvider):
	handle    = "freedns.afraid.org"
	name      = "freedns.afraid.org"
-- 
2.31.0




