From 8837c6299dfdbd082d1adb9055925e6a2591ff27 Mon Sep 17 00:00:00 2001
From: Jan Vansteenkiste <jan.vansteenkiste@ugent.be>
Date: Thu, 23 Jan 2025 23:20:56 +0100
Subject: [PATCH] collectd::plugin::modbus: reformat data type aliases and add
 spec tests

---
 .../type_aliases/collectd_modbus_data_spec.rb | 34 +++++++++++++++++++
 .../type_aliases/collectd_modbus_host_spec.rb | 19 +++++++++++
 .../collectd_modbus_slave_spec.rb             | 13 +++++++
 types/modbus/data.pp                          | 12 +++----
 types/modbus/host.pp                          |  9 +++--
 types/modbus/slave.pp                         | 10 ++++--
 6 files changed, 87 insertions(+), 10 deletions(-)
 create mode 100644 spec/type_aliases/collectd_modbus_data_spec.rb
 create mode 100644 spec/type_aliases/collectd_modbus_host_spec.rb
 create mode 100644 spec/type_aliases/collectd_modbus_slave_spec.rb

diff --git a/spec/type_aliases/collectd_modbus_data_spec.rb b/spec/type_aliases/collectd_modbus_data_spec.rb
new file mode 100644
index 00000000..86b5e9a1
--- /dev/null
+++ b/spec/type_aliases/collectd_modbus_data_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Collectd::Modbus::Data' do
+  it do
+    is_expected.to allow_values({
+                                  'type' => 'foo',
+                                  'register_base' => 123,
+                                  'register_type' => 'Int32',
+                                })
+  end
+
+  it do
+    is_expected.to allow_values({
+                                  'type' => 'foo',
+                                  'register_base' => 123,
+                                  'register_type' => 'Int32',
+                                  'register_cmd' => 'ReadInput',
+                                })
+  end
+
+  it do
+    is_expected.to allow_values({
+                                  'instance' => 'foobar',
+                                  'type' => 'foo',
+                                  'register_base' => 123,
+                                  'register_type' => 'Int32',
+                                })
+  end
+
+  it { is_expected.not_to allow_values(nil) }
+  it { is_expected.not_to allow_values({ 'type' => 'foo', 'register_base' => 123 }) }
+end
diff --git a/spec/type_aliases/collectd_modbus_host_spec.rb b/spec/type_aliases/collectd_modbus_host_spec.rb
new file mode 100644
index 00000000..99d9d5a4
--- /dev/null
+++ b/spec/type_aliases/collectd_modbus_host_spec.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Collectd::Modbus::Host' do
+  let(:slaves) do
+    {
+      1 => { 'instance' => 'foo1', 'collect' => 'bar1' },
+      2 => { 'instance' => 'foo2', 'collect' => %w[bar1 bar2] },
+    }
+  end
+
+  it { is_expected.to allow_values({ 'address' => '127.0.0.1', 'port' => 1234, 'slaves' => slaves }) }
+  it { is_expected.to allow_values({ 'address' => '127.0.0.1', 'port' => 1234, 'slaves' => slaves, 'interval' => 120 }) }
+
+  it { is_expected.not_to allow_values(nil) }
+  it { is_expected.not_to allow_values({ 'address' => '127.0.0.1', 'port' => '1234', 'slaves' => slaves, 'interval' => 120 }) }
+  it { is_expected.not_to allow_values({ 'port' => 1234, 'slaves' => slaves, 'interval' => 120 }) }
+end
diff --git a/spec/type_aliases/collectd_modbus_slave_spec.rb b/spec/type_aliases/collectd_modbus_slave_spec.rb
new file mode 100644
index 00000000..1e2fb332
--- /dev/null
+++ b/spec/type_aliases/collectd_modbus_slave_spec.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Collectd::Modbus::Slave' do
+  it { is_expected.to allow_values({ 'instance' => 'foo1', 'collect' => 'bar1' }) }
+  it { is_expected.to allow_values({ 'instance' => 'foo1', 'collect' => %w[bar1 bar2] }) }
+
+  it { is_expected.not_to allow_values(nil) }
+  it { is_expected.not_to allow_values({ 'collect' => ['bar1'] }) }
+  it { is_expected.not_to allow_values({ 'instance' => 'foo1' }) }
+  it { is_expected.not_to allow_values({ 'instance' => 'foo1', 'collect' => [] }) }
+end
diff --git a/types/modbus/data.pp b/types/modbus/data.pp
index f4278221..c3252fae 100644
--- a/types/modbus/data.pp
+++ b/types/modbus/data.pp
@@ -1,8 +1,8 @@
-#
+# @summary represents a modbus data entry
 type Collectd::Modbus::Data = Struct[{
-  Optional['instance']      => String,
-  NotUndef['type']          => String[1],
-  NotUndef['register_base'] => Integer[0],
-  NotUndef['register_type'] => Enum['Int16', 'Int32', 'Uint16', 'Uint32', 'Float'],
-  Optional['register_cmd']  => Enum['ReadHolding', 'ReadInput'],
+    Optional['instance']      => String,
+    NotUndef['type']          => String[1],
+    NotUndef['register_base'] => Integer[0],
+    NotUndef['register_type'] => Enum['Int16', 'Int32', 'Uint16', 'Uint32', 'Float'],
+    Optional['register_cmd']  => Enum['ReadHolding', 'ReadInput'],
 }]
diff --git a/types/modbus/host.pp b/types/modbus/host.pp
index 95122d43..b59eff92 100644
--- a/types/modbus/host.pp
+++ b/types/modbus/host.pp
@@ -1,2 +1,7 @@
-#
-type Collectd::Modbus::Host = Struct[{NotUndef['address'] => String[1], NotUndef['port'] => String[1], NotUndef['slaves'] => Hash[Integer, Collectd::Modbus::Slave], Optional['interval'] => Integer[0]}]
+# @summary represents a modbus host entry
+type Collectd::Modbus::Host = Struct[{
+    NotUndef['address'] => String[1],
+    NotUndef['port'] => Stdlib::Port,
+    NotUndef['slaves'] => Hash[Integer, Collectd::Modbus::Slave],
+    Optional['interval'] => Integer[0]
+}]
diff --git a/types/modbus/slave.pp b/types/modbus/slave.pp
index 844750ce..7e31e6d5 100644
--- a/types/modbus/slave.pp
+++ b/types/modbus/slave.pp
@@ -1,2 +1,8 @@
-#
-type Collectd::Modbus::Slave = Struct[{NotUndef['instance'] => String[1], NotUndef['collect'] => Variant[String[1], Array[String[1], 1]]}]
+# @summary Represents a modbus host's slave entry
+type Collectd::Modbus::Slave = Struct[{
+    NotUndef['instance'] => String[1],
+    NotUndef['collect'] => Variant[
+      String[1],
+      Array[String[1], 1]
+    ]
+}]