Fix bug on iteration within a span

The old code assumed that the Fused node only connects Fiber nodes. In a
sequence of Fused - Amplifier - Fused - Fiber, the Amplifier would be
included by a mistake. In addition, the code was not that easy to read,
and it just instantiated StopIteration without raising that (which would
be an error in a generator context). It was also rather strict, failing
if the iterator was requested for an "edge node" (a transponder), and
one of the exceptions was not actually an f-string.

Finally, the span_loss function would occasionally report wrong values
(for example in the provided test case, span_loss("fused7") would say 1
instead of 17).

Fix this by making sure that prev_node_generator() and
next_node_generator() never return anything but Fiber and Fused nodes,
which made it possible to simplify the span_loss() function. This should
now properly account for the accumulated losses of an arbitrary sequence
of Fiber and Fused elements.

I went over this a few times because set_egress_amplifier() calls
span_loss() on a *ROADM* node type. That does not make any sense, and
the code deals with this "properly" by returning a loss of 0 dB. It was
a bit confusing for me to see that it's actually OK to ask for a "span
loss" that's identified by a ROADM.

A side effect of this code is that Fused instances that are isolated
from the rest of the topology no longer raise an exception. I was
thinking about preserving this (because for GNPy, the only element with
no previous or no next nodes are the transceivers, but then Esther's
test topology contains an isolated `fused4` element. If we want to make
this strict, we can do that easily like this:

 --- a/gnpy/core/network.py
 +++ b/gnpy/core/network.py
 @@ -162,10 +162,12 @@ _fiber_fused_types = (elements.Fused, elements.Fiber)
  def prev_node_generator(network, node):
      """fused spans interest:
      iterate over all predecessors while they are Fused or Fiber type"""
      try:
          prev_node = next(network.predecessors(node))
      except StopIteration:
 -        return
 +        if isinstance(node, elements.Transceiver):
 +            return
 +        raise NetworkTopologyError(f'Node {node.uid} is not properly connected, please check network topology')
      if isinstance(prev_node, _fiber_fused_types) and isinstance(node, _fiber_fused_types):
          yield prev_node
          yield from prev_node_generator(network, prev_node)

Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Co-authored-by: Jan Kundrát <jan.kundrat@telecominfraproject.com>
Change-Id: I41a65e89eef763f82e41e52bc325ed2d488cb601

tests/test_network_functions.py

@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: BSD-3-Clause
+#
+# Copyright (C) 2020 Telecom Infra Project and GNPy contributors
+# see LICENSE.md for a list of contributors
+#
+
+from pathlib import Path
+import pytest
+from gnpy.core.network import span_loss
+from gnpy.tools.json_io import load_equipment, load_network
+
+
+TEST_DIR = Path(__file__).parent
+EQPT_FILENAME = TEST_DIR / 'data/eqpt_config.json'
+NETWORK_FILENAME = TEST_DIR / 'data/bugfixiteratortopo.json'
+
+
+@pytest.mark.parametrize("node, attenuation", [
+    # first fiber span
+    ['fiber1', 10.5],
+    ['fiber2', 10.5],
+    ['fused1', 10.5],
+    # second span
+    ['fiber3', 16.0],
+    # third span
+    ['fiber4', 16.0],
+    # direct link between a ROADM and an amplifier
+    ['fused5', 0],
+    # fourth span
+    ['fiber6', 17],
+    ['fused7', 17],
+    # not connected anywhere
+    ['fused4', 1],
+    # all other nodes
+    ['Site_A', 0],
+    ['nodeA', 0],
+    ['amp2', 0],
+    ['nodeC', 0],
+    ['Site_C', 0],
+    ['amp3', 0],
+    ['amp4', 0],
+    ['nodeB', 0],
+    ['Site_B', 0],
+])
+def test_span_loss(node, attenuation):
+    equipment = load_equipment(EQPT_FILENAME)
+    network = load_network(NETWORK_FILENAME, equipment)
+    for x in network.nodes():
+        if x.uid == node:
+            assert attenuation == span_loss(network, x)
+            return
+    assert not f'node "{node}" referenced from test but not found in the topology'  # pragma: no cover