routeros fixes and latency rounding

lib/towerops/monitoring/check.ex

@@ -27,7 +27,16 @@ defmodule Towerops.Monitoring.Check do
     check
     |> cast(attrs, [:device_id, :agent_token_id, :status, :response_time_ms, :checked_at])
     |> validate_required([:device_id, :status, :checked_at])
+    |> round_response_time()
     |> foreign_key_constraint(:device_id, match: :suffix)
     |> foreign_key_constraint(:agent_token_id, match: :suffix)
   end
+
+  defp round_response_time(changeset) do
+    case get_change(changeset, :response_time_ms) do
+      nil -> changeset
+      ms when is_float(ms) -> put_change(changeset, :response_time_ms, Float.round(ms, 1))
+      _ -> changeset
+    end
+  end
 end

lib/towerops/snmp/client.ex

@@ -392,9 +392,15 @@ defmodule Towerops.Snmp.Client do
     end
   end
 
-  # Private functions
+  @doc """
+  Returns true if Phoenix-side SNMP operations are disabled.
 
-  defp phoenix_snmp_disabled do
+  When true, all direct SNMP operations from Phoenix (get, walk, etc.) are blocked.
+  SNMP polling should be handled by connected agents instead.
+  Always returns false in test environment.
+  """
+  @spec phoenix_snmp_disabled() :: boolean()
+  def phoenix_snmp_disabled do
     # Only disable in non-test environments (allow tests to run normally)
     if Application.get_env(:towerops, :env) == :test do
       false

lib/towerops/workers/device_monitor_worker.ex

@@ -18,6 +18,7 @@ defmodule Towerops.Workers.DeviceMonitorWorker do
   alias Towerops.Alerts
   alias Towerops.Devices
   alias Towerops.Monitoring
+  alias Towerops.Snmp.Client
   alias Towerops.Workers.PollingOffset
 
   require Logger
@@ -52,13 +53,17 @@ defmodule Towerops.Workers.DeviceMonitorWorker do
   end
 
   # Check if device should continue being monitored
-  # Returns false if monitoring is disabled or device is assigned to an agent
+  # Returns false if Phoenix SNMP is disabled, monitoring is disabled, or device is assigned to an agent
   defp should_continue_monitoring?(device) do
-    device.monitoring_enabled && Agents.should_phoenix_poll_device?(device)
+    !Client.phoenix_snmp_disabled() && device.monitoring_enabled &&
+      Agents.should_phoenix_poll_device?(device)
   end
 
   defp maybe_perform_check(device) do
     cond do
+      Client.phoenix_snmp_disabled() ->
+        :ok
+
       not device.monitoring_enabled ->
         :ok
 

lib/towerops/workers/device_poller_worker.ex

@@ -69,13 +69,17 @@ defmodule Towerops.Workers.DevicePollerWorker do
   end
 
   # Check if device should continue being polled
-  # Returns false if SNMP is disabled or device is assigned to an agent
+  # Returns false if Phoenix SNMP is disabled, SNMP is disabled on device, or device is assigned to an agent
   defp should_continue_polling?(device) do
-    device.snmp_enabled && Agents.should_phoenix_poll_device?(device)
+    !Client.phoenix_snmp_disabled() && device.snmp_enabled &&
+      Agents.should_phoenix_poll_device?(device)
   end
 
   defp maybe_poll_device(device) do
     cond do
+      Client.phoenix_snmp_disabled() ->
+        :ok
+
       not device.snmp_enabled ->
         :ok
 

lib/towerops_web/channels/agent_channel.ex

@@ -109,9 +109,8 @@ defmodule ToweropsWeb.AgentChannel do
         socket = assign(socket, :last_heartbeat_at, DateTime.utc_now())
         Process.send_after(self(), :check_heartbeat, @heartbeat_check_interval_ms)
 
-        # Send initial job list synchronously during join to avoid race condition
-        # where agent disconnects before the async :send_jobs message is processed
-        build_and_push_jobs(socket)
+        # Send initial job list after join completes (push/3 cannot be called during join)
+        send(self(), :send_jobs)
 
         {:ok, socket}
 

priv/profiles/os_discovery/routeros.yaml

@@ -28,6 +28,7 @@ modules:
                     num_oid: '.1.3.6.1.4.1.14988.1.1.3.100.1.3.{{ $index }}'
                     descr: MIKROTIK-MIB::mtxrGaugeName
                     group: System
+                    divisor: 10
                     index: 'mtxrGaugeTemperature.{{ $index }}'
                     skip_values:
                         -   oid: MIKROTIK-MIB::mtxrGaugeUnit

priv/profiles/os_discovery/swos.yaml

@@ -70,6 +70,7 @@ modules:
                     num_oid: '.1.3.6.1.4.1.14988.1.1.3.100.1.3.{{ $index }}'
                     descr: 'CPU Temperature'
                     group: System
+                    divisor: 10
                     index: 'mtxrGaugeTemperature.{{ $index }}'
         voltage:
             data:

test/integration/discovery_parity_test.exs

@@ -0,0 +1,991 @@
+defmodule Towerops.Integration.DiscoveryParityTest do
+  @moduledoc """
+  Integration tests for SNMP discovery/polling parity.
+
+  Runs against a real SNMP device (MikroTik RB5009UG+S+) using the actual
+  SnmpKit library (not mocks) and verifies our discovery pipeline captures
+  everything that snmpwalk/snmpget returns.
+
+  These tests are excluded from normal `mix test` runs via the :integration tag.
+  Run with: mix test test/integration/discovery_parity_test.exs --include integration
+  """
+  use Towerops.DataCase, async: false
+
+  import Towerops.DevicesFixtures
+
+  alias Towerops.Snmp
+  alias Towerops.Snmp.Client
+  alias Towerops.Snmp.Discovery
+  alias Towerops.Snmp.Profiles.Base
+  alias Towerops.Snmp.Profiles.Dynamic
+
+  require Logger
+
+  @moduletag :integration
+
+  # Test device: MikroTik RB5009UG+S+ running RouterOS
+  @test_ip "204.110.191.1"
+  @test_community "testlocal"
+  @test_version "2c"
+
+  @client_opts [
+    ip: @test_ip,
+    community: @test_community,
+    version: @test_version,
+    port: 161,
+    timeout: 10_000
+  ]
+
+  # Standard OIDs for ground truth comparisons
+  @sys_uptime_oid "1.3.6.1.2.1.1.3.0"
+  @if_index_oid "1.3.6.1.2.1.2.2.1.1"
+  @if_descr_oid "1.3.6.1.2.1.2.2.1.2"
+  @if_type_oid "1.3.6.1.2.1.2.2.1.3"
+  @if_speed_oid "1.3.6.1.2.1.2.2.1.5"
+  @if_phys_address_oid "1.3.6.1.2.1.2.2.1.6"
+  @if_admin_status_oid "1.3.6.1.2.1.2.2.1.7"
+  @if_oper_status_oid "1.3.6.1.2.1.2.2.1.8"
+  @if_name_oid "1.3.6.1.2.1.31.1.1.1.1"
+  @if_high_speed_oid "1.3.6.1.2.1.31.1.1.1.15"
+  @if_alias_oid "1.3.6.1.2.1.31.1.1.1.18"
+  @if_in_octets_oid "1.3.6.1.2.1.2.2.1.10"
+  @if_out_octets_oid "1.3.6.1.2.1.2.2.1.16"
+  @entity_sensor_type_oid "1.3.6.1.2.1.99.1.1.1.1"
+  @entity_phys_descr_oid "1.3.6.1.2.1.47.1.1.1.1.2"
+
+  # IP-MIB OIDs
+  @ip_ad_ent_addr_oid "1.3.6.1.2.1.4.20.1.1"
+  @ip_ad_ent_if_index_oid "1.3.6.1.2.1.4.20.1.2"
+  @ip_ad_ent_net_mask_oid "1.3.6.1.2.1.4.20.1.3"
+
+  # HR-MIB OIDs
+  @hr_processor_load_oid "1.3.6.1.2.1.25.3.3.1.2"
+  @hr_storage_descr_oid "1.3.6.1.2.1.25.2.3.1.3"
+  @hr_storage_size_oid "1.3.6.1.2.1.25.2.3.1.5"
+  @hr_storage_used_oid "1.3.6.1.2.1.25.2.3.1.6"
+
+  # MikroTik enterprise OIDs
+  @mtxr_gauge_name_oid "1.3.6.1.4.1.14988.1.1.3.100.1.2"
+  @mtxr_gauge_value_oid "1.3.6.1.4.1.14988.1.1.3.100.1.3"
+  @mtxr_serial_oid "1.3.6.1.4.1.14988.1.1.4.1.0"
+  @mtxr_firmware_oid "1.3.6.1.4.1.14988.1.1.4.4.0"
+  @mtxr_model_oid "1.3.6.1.4.1.14988.1.1.7.8.0"
+  @mtxr_dhcp_lease_count_oid "1.3.6.1.4.1.14988.1.1.6.1.0"
+  @mtxr_hl_voltage_oid "1.3.6.1.4.1.14988.1.1.3.11.0"
+
+  setup do
+    # Override SNMP adapter from mock to real SnmpKit
+    original_adapter = Application.get_env(:towerops, :snmp_adapter)
+    Application.put_env(:towerops, :snmp_adapter, SnmpKit)
+
+    # Check device connectivity with a quick sysUpTime GET
+    reachable =
+      case Client.get(@client_opts, @sys_uptime_oid) do
+        {:ok, _} ->
+          true
+
+        {:error, reason} ->
+          Logger.warning("Test device #{@test_ip} unreachable: #{inspect(reason)}")
+          false
+      end
+
+    on_exit(fn ->
+      if original_adapter do
+        Application.put_env(:towerops, :snmp_adapter, original_adapter)
+      else
+        Application.delete_env(:towerops, :snmp_adapter)
+      end
+    end)
+
+    %{reachable: reachable}
+  end
+
+  describe "raw SNMP connectivity" do
+    @tag timeout: 30_000
+    test "GET sysUpTime.0 returns integer", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, uptime} = Client.get(@client_opts, @sys_uptime_oid)
+      assert is_integer(uptime), "Expected integer uptime, got: #{inspect(uptime)}"
+      assert uptime >= 0
+    end
+
+    @tag timeout: 60_000
+    test "WALK ifIndex returns non-empty results", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, results} = Client.walk(@client_opts, @if_index_oid)
+      assert is_map(results)
+      assert map_size(results) > 0, "Expected at least one interface"
+
+      for {_oid, value} <- results do
+        assert is_integer(value) and value > 0,
+               "ifIndex values must be positive integers, got: #{inspect(value)}"
+      end
+    end
+  end
+
+  describe "system info parity" do
+    @tag timeout: 60_000
+    test "discover_system_info returns all SNMPv2-MIB fields", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, system_info} = Base.discover_system_info(@client_opts)
+
+      # All 6 standard system fields should be present and populated
+      assert is_binary(system_info.sys_descr)
+      assert String.length(system_info.sys_descr) > 0
+
+      assert is_binary(system_info.sys_object_id)
+      assert String.starts_with?(system_info.sys_object_id, "1.3.6.1")
+
+      assert is_binary(system_info.sys_name)
+      assert String.length(system_info.sys_name) > 0
+
+      assert is_integer(system_info.sys_uptime)
+      assert system_info.sys_uptime >= 0
+
+      # Contact and location should be present (may be empty strings)
+      assert Map.has_key?(system_info, :sys_contact)
+      assert Map.has_key?(system_info, :sys_location)
+
+      Logger.info(
+        "System: descr=#{String.slice(system_info.sys_descr, 0..80)}, " <>
+          "name=#{system_info.sys_name}, oid=#{system_info.sys_object_id}, " <>
+          "contact=#{system_info.sys_contact}, location=#{system_info.sys_location}"
+      )
+    end
+
+    @tag timeout: 60_000
+    test "raw SNMP values match discovery output", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, raw_descr} = Client.get(@client_opts, "1.3.6.1.2.1.1.1.0")
+      {:ok, raw_name} = Client.get(@client_opts, "1.3.6.1.2.1.1.5.0")
+      {:ok, raw_contact} = Client.get(@client_opts, "1.3.6.1.2.1.1.4.0")
+      {:ok, raw_location} = Client.get(@client_opts, "1.3.6.1.2.1.1.6.0")
+      {:ok, raw_object_id} = Client.get(@client_opts, "1.3.6.1.2.1.1.2.0")
+
+      {:ok, system_info} = Base.discover_system_info(@client_opts)
+
+      assert is_binary(raw_descr)
+      assert is_binary(system_info.sys_descr)
+      assert system_info.sys_name == to_string(raw_name)
+
+      # sysObjectID comes as OID list from raw SNMP, discovery converts to dotted string
+      assert is_list(raw_object_id) or is_binary(raw_object_id)
+      assert is_binary(system_info.sys_object_id)
+      assert String.starts_with?(system_info.sys_object_id, "1.3.6.1")
+
+      # Contact and location should match raw values
+      assert system_info.sys_contact == to_string(raw_contact)
+      assert system_info.sys_location == to_string(raw_location)
+    end
+  end
+
+  describe "profile selection" do
+    @tag timeout: 60_000
+    test "selects RouterOS YAML profile for MikroTik device", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, system_info} = Base.discover_system_info(@client_opts)
+      profile = Discovery.select_profile(system_info, @client_opts)
+
+      # This device has sysObjectID .1.3.6.1.4.1.14988.1 which should match routeros profile
+      assert match?({:yaml, %{name: _}}, profile),
+             "Expected YAML profile match for MikroTik device, got: #{inspect(profile)}"
+
+      {:yaml, yaml_profile} = profile
+      Logger.info("Selected YAML profile: #{yaml_profile.name}")
+
+      # Verify profile has sensor definitions (routeros.yaml has temperature, voltage, etc.)
+      assert Map.has_key?(yaml_profile, :sensors) or Map.has_key?(yaml_profile, :table_sensor_oids),
+             "RouterOS profile should have sensor definitions"
+    end
+  end
+
+  describe "interface discovery parity" do
+    @tag timeout: 120_000
+    test "interface count matches raw ifIndex walk", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, raw_if_indices} = Client.walk(@client_opts, @if_index_oid)
+      expected_count = map_size(raw_if_indices)
+
+      {:ok, interfaces} = Base.discover_interfaces(@client_opts)
+
+      assert length(interfaces) == expected_count,
+             "Discovery found #{length(interfaces)} interfaces, " <>
+               "raw walk found #{expected_count}"
+
+      Logger.info("Interface count matches: #{expected_count}")
+    end
+
+    @tag timeout: 120_000
+    test "ifIndex values match raw walk exactly", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, raw_if_indices} = Client.walk(@client_opts, @if_index_oid)
+      {:ok, interfaces} = Base.discover_interfaces(@client_opts)
+
+      raw_indices = raw_if_indices |> Map.values() |> MapSet.new()
+      discovered_indices = MapSet.new(interfaces, & &1.if_index)
+
+      assert MapSet.equal?(raw_indices, discovered_indices),
+             "ifIndex mismatch - raw: #{inspect(MapSet.to_list(raw_indices))}, " <>
+               "discovered: #{inspect(MapSet.to_list(discovered_indices))}"
+    end
+
+    @tag timeout: 120_000
+    test "all ifTable fields populated per interface", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      # Get raw ground truth for all standard IF-MIB fields
+      {:ok, raw_descrs} = Client.walk(@client_opts, @if_descr_oid)
+      {:ok, raw_types} = Client.walk(@client_opts, @if_type_oid)
+      {:ok, raw_admin} = Client.walk(@client_opts, @if_admin_status_oid)
+      {:ok, raw_oper} = Client.walk(@client_opts, @if_oper_status_oid)
+
+      {:ok, interfaces} = Base.discover_interfaces(@client_opts)
+
+      # Count matches for all fields
+      assert length(interfaces) == map_size(raw_descrs),
+             "ifDescr count mismatch: #{length(interfaces)} vs #{map_size(raw_descrs)}"
+
+      assert length(interfaces) == map_size(raw_types),
+             "ifType count mismatch: #{length(interfaces)} vs #{map_size(raw_types)}"
+
+      assert length(interfaces) == map_size(raw_admin),
+             "ifAdminStatus count mismatch: #{length(interfaces)} vs #{map_size(raw_admin)}"
+
+      assert length(interfaces) == map_size(raw_oper),
+             "ifOperStatus count mismatch: #{length(interfaces)} vs #{map_size(raw_oper)}"
+
+      for iface <- interfaces do
+        # ifDescr must be present on every interface
+        assert is_binary(iface.if_descr),
+               "Interface #{iface.if_index} missing if_descr"
+
+        assert String.length(iface.if_descr) > 0,
+               "Interface #{iface.if_index} has empty if_descr"
+
+        # ifType must be a positive integer (IANAifType)
+        assert is_integer(iface.if_type) and iface.if_type > 0,
+               "Interface #{iface.if_index} invalid ifType: #{inspect(iface.if_type)}"
+
+        # Admin/oper status - device returns string atoms or integers
+        valid_admin = [1, 2, 3, "up", "down", "testing"]
+
+        valid_oper = [
+          1,
+          2,
+          3,
+          4,
+          5,
+          6,
+          7,
+          "up",
+          "down",
+          "testing",
+          "unknown",
+          "dormant",
+          "notPresent",
+          "lowerLayerDown"
+        ]
+
+        assert iface.if_admin_status in valid_admin,
+               "Interface #{iface.if_index} invalid admin status: #{inspect(iface.if_admin_status)}"
+
+        assert iface.if_oper_status in valid_oper,
+               "Interface #{iface.if_index} invalid oper status: #{inspect(iface.if_oper_status)}"
+      end
+    end
+
+    @tag timeout: 120_000
+    test "ifXTable extended fields populated", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      # Ground truth for ifXTable
+      {:ok, raw_names} = Client.walk(@client_opts, @if_name_oid)
+      {:ok, raw_aliases} = Client.walk(@client_opts, @if_alias_oid)
+      {:ok, raw_high_speed} = Client.walk(@client_opts, @if_high_speed_oid)
+      {:ok, raw_speeds} = Client.walk(@client_opts, @if_speed_oid)
+
+      {:ok, interfaces} = Base.discover_interfaces(@client_opts)
+
+      # ifName should be discovered for all interfaces
+      assert map_size(raw_names) == length(interfaces),
+             "Raw ifName count (#{map_size(raw_names)}) != interface count (#{length(interfaces)})"
+
+      interfaces_with_name = Enum.count(interfaces, & &1[:if_name])
+
+      Logger.info(
+        "ifXTable: #{interfaces_with_name}/#{length(interfaces)} have ifName, " <>
+          "#{map_size(raw_aliases)} have ifAlias, " <>
+          "#{map_size(raw_high_speed)} have ifHighSpeed, " <>
+          "#{map_size(raw_speeds)} have ifSpeed"
+      )
+
+      assert interfaces_with_name == map_size(raw_names),
+             "Discovery missed #{map_size(raw_names) - interfaces_with_name} ifName entries"
+
+      # Verify ifPhysAddress (MAC address) is populated where available
+      {:ok, raw_macs} = Client.walk(@client_opts, @if_phys_address_oid)
+
+      interfaces_with_mac =
+        Enum.count(interfaces, fn iface ->
+          iface[:if_phys_address] != nil and iface[:if_phys_address] != ""
+        end)
+
+      # Count non-empty MACs from raw walk (PPP interfaces have empty MACs)
+      raw_nonempty_macs =
+        Enum.count(raw_macs, fn {_oid, val} ->
+          val != "" and val != nil and val != <<0, 0, 0, 0, 0, 0>>
+        end)
+
+      Logger.info("MAC addresses: #{interfaces_with_mac} discovered, #{raw_nonempty_macs} non-empty in raw walk")
+    end
+  end
+
+  describe "sensor discovery parity" do
+    @tag timeout: 120_000
+    test "ENTITY-SENSOR-MIB sensor count matches raw walk", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      case Client.walk(@client_opts, @entity_sensor_type_oid) do
+        {:ok, raw_sensors} when map_size(raw_sensors) > 0 ->
+          expected_count = map_size(raw_sensors)
+          {:ok, sensors} = Base.discover_sensors(@client_opts)
+
+          Logger.info("ENTITY-SENSOR-MIB: raw=#{expected_count}, discovered=#{length(sensors)}")
+
+          assert length(sensors) == expected_count,
+                 "Sensor count mismatch: raw=#{expected_count}, discovered=#{length(sensors)}"
+
+        {:ok, raw_sensors} when map_size(raw_sensors) == 0 ->
+          Logger.info("Device has no ENTITY-SENSOR-MIB sensors (expected for MikroTik)")
+          {:ok, sensors} = Base.discover_sensors(@client_opts)
+          assert sensors == []
+
+        {:error, _reason} ->
+          Logger.info("ENTITY-SENSOR-MIB not supported (expected for MikroTik)")
+          {:ok, sensors} = Base.discover_sensors(@client_opts)
+          assert is_list(sensors)
+      end
+    end
+
+    @tag timeout: 120_000
+    test "MikroTik gauge sensors discovered via YAML profile", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      # Ground truth: raw MikroTik gauge table
+      {:ok, raw_gauge_names} = Client.walk(@client_opts, @mtxr_gauge_name_oid)
+      {:ok, raw_gauge_values} = Client.walk(@client_opts, @mtxr_gauge_value_oid)
+
+      Logger.info(
+        "Raw MikroTik gauges: #{map_size(raw_gauge_names)} names, " <>
+          "#{map_size(raw_gauge_values)} values"
+      )
+
+      for {oid, name} <- raw_gauge_names do
+        index = oid |> String.split(".") |> List.last()
+        value_oid = @mtxr_gauge_value_oid <> ".#{index}"
+        value = Map.get(raw_gauge_values, value_oid)
+        Logger.info("  Gauge #{index}: name=#{inspect(name)}, value=#{inspect(value)}")
+      end
+
+      # Now check that YAML profile discovery finds these
+      {:ok, system_info} = Base.discover_system_info(@client_opts)
+      profile = Discovery.select_profile(system_info, @client_opts)
+
+      assert match?({:yaml, _}, profile), "Expected YAML profile for MikroTik"
+      {:yaml, yaml_profile} = profile
+
+      {:ok, yaml_sensors} = Dynamic.discover_sensors(yaml_profile, @client_opts)
+
+      # Log all discovered sensors for comparison
+      Logger.info("YAML profile discovered #{length(yaml_sensors)} sensors:")
+
+      for sensor <- yaml_sensors do
+        Logger.info(
+          "  #{sensor.sensor_type}/#{sensor.sensor_index}: " <>
+            "#{sensor.sensor_descr} = #{inspect(sensor[:last_value])} " <>
+            "(oid=#{sensor.sensor_oid}, unit=#{sensor.sensor_unit}, div=#{sensor.sensor_divisor})"
+        )
+      end
+
+      # MikroTik gauges should produce at least one temperature sensor
+      temp_sensors = Enum.filter(yaml_sensors, &(&1.sensor_type == "temperature"))
+
+      assert temp_sensors != [],
+             "Expected MikroTik temperature sensors (mtxrGaugeTable has cpu-temperature)"
+
+      # Verify raw gauge value matches a discovered sensor
+      if map_size(raw_gauge_names) > 0 do
+        gauge_sensor_oids =
+          yaml_sensors
+          |> Enum.map(& &1.sensor_oid)
+          |> Enum.filter(&String.contains?(&1, "14988.1.1.3.100"))
+
+        assert gauge_sensor_oids != [],
+               "Expected sensors from mtxrGaugeTable (1.3.6.1.4.1.14988.1.1.3.100)"
+      end
+    end
+
+    @tag timeout: 120_000
+    test "MikroTik voltage sensors discovered", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      # Raw voltage value
+      case Client.get(@client_opts, @mtxr_hl_voltage_oid) do
+        {:ok, raw_voltage} ->
+          Logger.info("Raw mtxrHlVoltage: #{inspect(raw_voltage)}")
+
+          {:ok, system_info} = Base.discover_system_info(@client_opts)
+          {:yaml, yaml_profile} = Discovery.select_profile(system_info, @client_opts)
+          {:ok, sensors} = Dynamic.discover_sensors(yaml_profile, @client_opts)
+
+          voltage_sensors = Enum.filter(sensors, &(&1.sensor_type == "voltage"))
+
+          Logger.info("Voltage sensors discovered: #{length(voltage_sensors)}")
+
+          for vs <- voltage_sensors do
+            Logger.info("  #{vs.sensor_descr}: value=#{inspect(vs[:last_value])}, oid=#{vs.sensor_oid}")
+          end
+
+          assert voltage_sensors != [], "Expected voltage sensors from MikroTik"
+
+        {:error, reason} ->
+          Logger.warning("mtxrHlVoltage not available: #{inspect(reason)}")
+      end
+    end
+
+    @tag timeout: 120_000
+    test "MikroTik count sensors discovered (DHCP, firewall)", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      # Check raw DHCP lease count
+      case Client.get(@client_opts, @mtxr_dhcp_lease_count_oid) do
+        {:ok, raw_dhcp} ->
+          Logger.info("Raw mtxrDHCPLeaseCount: #{inspect(raw_dhcp)}")
+
+        {:error, reason} ->
+          Logger.warning("mtxrDHCPLeaseCount not available: #{inspect(reason)}")
+      end
+
+      {:ok, system_info} = Base.discover_system_info(@client_opts)
+      {:yaml, yaml_profile} = Discovery.select_profile(system_info, @client_opts)
+      {:ok, sensors} = Dynamic.discover_sensors(yaml_profile, @client_opts)
+
+      count_sensors = Enum.filter(sensors, &(&1.sensor_type == "count"))
+
+      Logger.info("Count sensors discovered: #{length(count_sensors)}")
+
+      for cs <- count_sensors do
+        Logger.info("  #{cs.sensor_descr}: value=#{inspect(cs[:last_value])}, oid=#{cs.sensor_oid}")
+      end
+
+      # MikroTik should have DHCP lease count and/or firewall connection counts
+      assert count_sensors != [],
+             "Expected count sensors (DHCP leases, firewall connections)"
+    end
+
+    @tag timeout: 120_000
+    test "all discovered sensor fields have valid types", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, system_info} = Base.discover_system_info(@client_opts)
+      profile = Discovery.select_profile(system_info, @client_opts)
+
+      sensors =
+        case profile do
+          {:yaml, yaml_profile} ->
+            {:ok, s} = Dynamic.discover_sensors(yaml_profile, @client_opts)
+            s
+
+          Base ->
+            {:ok, s} = Base.discover_sensors(@client_opts)
+            s
+        end
+
+      for sensor <- sensors do
+        assert is_binary(sensor.sensor_type), "sensor_type must be string: #{inspect(sensor)}"
+        assert is_binary(sensor.sensor_index), "sensor_index must be string: #{inspect(sensor)}"
+        assert is_binary(sensor.sensor_oid), "sensor_oid must be string: #{inspect(sensor)}"
+
+        assert String.starts_with?(sensor.sensor_oid, "1.3.6.1") or
+                 String.starts_with?(sensor.sensor_oid, ".1.3.6.1"),
+               "sensor_oid must be valid OID: #{sensor.sensor_oid}"
+
+        assert is_binary(sensor.sensor_descr), "sensor_descr must be string: #{inspect(sensor)}"
+        assert is_binary(sensor.sensor_unit), "sensor_unit must be string: #{inspect(sensor)}"
+        assert is_number(sensor.sensor_divisor), "sensor_divisor must be number: #{inspect(sensor)}"
+        assert sensor.sensor_divisor > 0, "sensor_divisor must be positive: #{sensor.sensor_divisor}"
+      end
+
+      # Log sensor type breakdown
+      by_type = Enum.group_by(sensors, & &1.sensor_type)
+
+      for {type, group} <- by_type do
+        Logger.info("  #{type}: #{length(group)} sensors")
+      end
+
+      Logger.info("Validated #{length(sensors)} sensors across #{map_size(by_type)} types")
+    end
+  end
+
+  describe "device identification parity" do
+    @tag timeout: 120_000
+    test "MikroTik-specific OIDs provide serial, firmware, model", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      # Raw ground truth from MikroTik enterprise OIDs
+      {:ok, raw_serial} = Client.get(@client_opts, @mtxr_serial_oid)
+      {:ok, raw_firmware} = Client.get(@client_opts, @mtxr_firmware_oid)
+      {:ok, raw_model} = Client.get(@client_opts, @mtxr_model_oid)
+
+      Logger.info(
+        "Raw MikroTik: serial=#{inspect(raw_serial)}, " <>
+          "firmware=#{inspect(raw_firmware)}, model=#{inspect(raw_model)}"
+      )
+
+      assert is_binary(raw_serial) and String.length(raw_serial) > 0
+      assert is_binary(raw_firmware) and String.length(raw_firmware) > 0
+      assert is_binary(raw_model) and String.length(raw_model) > 0
+
+      # Now verify discovery captures these
+      {:ok, system_info} = Base.discover_system_info(@client_opts)
+      {:yaml, yaml_profile} = Discovery.select_profile(system_info, @client_opts)
+      device_info = Dynamic.identify_device(yaml_profile, @client_opts, system_info)
+
+      Logger.info(
+        "Discovery device_info: manufacturer=#{inspect(device_info[:manufacturer])}, " <>
+          "model=#{inspect(device_info[:model])}, " <>
+          "firmware=#{inspect(device_info[:firmware_version])}, " <>
+          "serial=#{inspect(device_info[:serial_number])}"
+      )
+
+      assert is_binary(device_info.manufacturer)
+      assert is_binary(device_info.model)
+
+      # Serial and firmware should come from MikroTik-specific OIDs
+      if device_info[:serial_number] do
+        assert is_binary(device_info.serial_number)
+        assert String.length(device_info.serial_number) > 0
+        Logger.info("Serial number captured: #{device_info.serial_number}")
+      else
+        Logger.warning("Serial number NOT captured from MikroTik OIDs (parity gap)")
+      end
+
+      if device_info[:firmware_version] do
+        assert is_binary(device_info.firmware_version)
+        assert String.length(device_info.firmware_version) > 0
+        Logger.info("Firmware version captured: #{device_info.firmware_version}")
+      else
+        Logger.warning("Firmware version NOT captured from MikroTik OIDs (parity gap)")
+      end
+    end
+  end
+
+  describe "ENTITY-MIB physical entities" do
+    @tag timeout: 120_000
+    test "physical entity descriptions discovered", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, raw_entities} = Client.walk(@client_opts, @entity_phys_descr_oid)
+
+      Logger.info("ENTITY-MIB physical entities: #{map_size(raw_entities)}")
+
+      for {oid, descr} <- raw_entities do
+        index = oid |> String.split(".") |> List.last()
+        Logger.info("  Entity #{index}: #{inspect(descr)}")
+      end
+
+      # Verify our discovery can read these too
+      {:ok, entities} = Base.discover_physical_entities(@client_opts)
+      assert is_list(entities)
+
+      Logger.info("Discovery found #{length(entities)} physical entities")
+
+      # MikroTik has ENTITY-MIB entries (RouterOS info, USB controllers, etc.)
+      if map_size(raw_entities) > 0 do
+        assert entities != [],
+               "Expected physical entities - raw walk found #{map_size(raw_entities)}"
+      end
+    end
+  end
+
+  describe "secondary discovery parity" do
+    @tag timeout: 120_000
+    test "VLANs from Q-BRIDGE-MIB", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, vlans} = Base.discover_vlans(@client_opts)
+      assert is_list(vlans)
+
+      Logger.info("Discovered #{length(vlans)} VLANs (MikroTik typically returns 0)")
+    end
+
+    @tag timeout: 120_000
+    test "IP addresses match raw IP-MIB walk", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      # Raw ground truth
+      {:ok, raw_ip_addrs} = Client.walk(@client_opts, @ip_ad_ent_addr_oid)
+      {:ok, raw_ip_if_index} = Client.walk(@client_opts, @ip_ad_ent_if_index_oid)
+      {:ok, raw_ip_masks} = Client.walk(@client_opts, @ip_ad_ent_net_mask_oid)
+
+      Logger.info(
+        "Raw IP-MIB: #{map_size(raw_ip_addrs)} addresses, " <>
+          "#{map_size(raw_ip_if_index)} if-indices, #{map_size(raw_ip_masks)} netmasks"
+      )
+
+      for {oid, addr} <- raw_ip_addrs do
+        index = String.replace_prefix(oid, "1.3.6.1.2.1.4.20.1.1.", "")
+        if_idx_oid = "1.3.6.1.2.1.4.20.1.2.#{index}"
+        mask_oid = "1.3.6.1.2.1.4.20.1.3.#{index}"
+        if_idx = Map.get(raw_ip_if_index, if_idx_oid)
+        mask = Map.get(raw_ip_masks, mask_oid)
+        Logger.info("  #{inspect(addr)} (ifIndex=#{inspect(if_idx)}, mask=#{inspect(mask)})")
+      end
+
+      # Discovery output
+      {:ok, ip_addresses} = Base.discover_all_ip_addresses(@client_opts)
+      assert is_list(ip_addresses)
+
+      # Should discover at least as many IPs as raw walk
+      assert length(ip_addresses) >= map_size(raw_ip_addrs),
+             "Discovery found #{length(ip_addresses)} IPs, raw walk found #{map_size(raw_ip_addrs)}"
+
+      # Target IP must be among discovered addresses
+      all_ips = Enum.map(ip_addresses, & &1[:ip_address])
+
+      Logger.info("Discovered IPs: #{inspect(all_ips)}")
+
+      assert @test_ip in all_ips,
+             "Target IP #{@test_ip} not found among discovered IPs: #{inspect(all_ips)}"
+    end
+
+    @tag timeout: 120_000
+    test "processors match raw HR-MIB walk", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, raw_proc_loads} = Client.walk(@client_opts, @hr_processor_load_oid)
+
+      Logger.info("Raw HR-MIB processors: #{map_size(raw_proc_loads)}")
+
+      for {oid, load} <- raw_proc_loads do
+        index = oid |> String.split(".") |> List.last()
+        Logger.info("  Processor #{index}: load=#{load}%")
+      end
+
+      {:ok, processors} = Base.discover_processors(@client_opts)
+      assert is_list(processors)
+
+      # Count should match raw walk
+      assert length(processors) == map_size(raw_proc_loads),
+             "Processor count mismatch: discovered=#{length(processors)}, " <>
+               "raw=#{map_size(raw_proc_loads)}"
+
+      for proc <- processors do
+        assert is_map(proc)
+
+        if proc[:load] do
+          assert is_number(proc.load)
+
+          assert proc.load >= 0 and proc.load <= 100,
+                 "CPU load out of range: #{proc.load}"
+        end
+      end
+
+      Logger.info("Discovered #{length(processors)} processors (matches raw walk)")
+    end
+
+    @tag timeout: 120_000
+    test "storage matches raw HR-MIB walk (disk types only)", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      # Raw HR-MIB includes ALL storage types: RAM, FixedDisk, Other, etc.
+      {:ok, raw_storage_descr} = Client.walk(@client_opts, @hr_storage_descr_oid)
+      {:ok, raw_storage_size} = Client.walk(@client_opts, @hr_storage_size_oid)
+      {:ok, raw_storage_used} = Client.walk(@client_opts, @hr_storage_used_oid)
+      {:ok, raw_storage_types} = Client.walk(@client_opts, "1.3.6.1.2.1.25.2.3.1.2")
+
+      Logger.info("Raw HR-MIB storage entries (all types): #{map_size(raw_storage_descr)}")
+
+      # Disk-type OIDs that discover_storage keeps (RAM and Other are filtered out)
+      disk_type_oids = [
+        # fixedDisk
+        [1, 3, 6, 1, 2, 1, 25, 2, 1, 4],
+        # removableDisk
+        [1, 3, 6, 1, 2, 1, 25, 2, 1, 5],
+        # floppyDisk
+        [1, 3, 6, 1, 2, 1, 25, 2, 1, 6],
+        # compactDisc
+        [1, 3, 6, 1, 2, 1, 25, 2, 1, 7],
+        # ramDisk
+        [1, 3, 6, 1, 2, 1, 25, 2, 1, 8],
+        # flashMemory
+        [1, 3, 6, 1, 2, 1, 25, 2, 1, 9],
+        # networkDisk
+        [1, 3, 6, 1, 2, 1, 25, 2, 1, 10]
+      ]
+
+      raw_disk_count =
+        Enum.count(raw_storage_types, fn {_oid, type_val} ->
+          type_val in disk_type_oids
+        end)
+
+      for {oid, descr} <- raw_storage_descr do
+        index = oid |> String.split(".") |> List.last()
+        type_oid = "1.3.6.1.2.1.25.2.3.1.2.#{index}"
+        size_oid = "1.3.6.1.2.1.25.2.3.1.5.#{index}"
+        used_oid = "1.3.6.1.2.1.25.2.3.1.6.#{index}"
+        type_val = Map.get(raw_storage_types, type_oid)
+        is_disk = type_val in disk_type_oids
+        size = Map.get(raw_storage_size, size_oid)
+        used = Map.get(raw_storage_used, used_oid)
+
+        Logger.info(
+          "  #{inspect(descr)}: type=#{inspect(type_val)}, disk=#{is_disk}, " <>
+            "size=#{inspect(size)}, used=#{inspect(used)}"
+        )
+      end
+
+      {:ok, storage} = Base.discover_storage(@client_opts)
+      assert is_list(storage)
+
+      # discover_storage only returns disk-type entries (not RAM or Other)
+      assert length(storage) == raw_disk_count,
+             "Storage count mismatch: discovered=#{length(storage)}, " <>
+               "raw disk-type=#{raw_disk_count} (total raw=#{map_size(raw_storage_descr)})"
+
+      for entry <- storage do
+        assert is_map(entry)
+        assert entry[:total_bytes] != nil, "Storage entry missing total_bytes"
+        assert is_number(entry.total_bytes)
+        assert entry.total_bytes >= 0
+      end
+
+      Logger.info(
+        "Discovered #{length(storage)} disk storage entries (raw has #{raw_disk_count} disk, #{map_size(raw_storage_descr)} total)"
+      )
+    end
+  end
+
+  describe "full discovery pipeline" do
+    @tag timeout: 300_000
+    test "discover_device saves complete data to DB", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      device =
+        device_fixture(%{
+          ip_address: @test_ip,
+          snmp_enabled: true,
+          snmp_version: @test_version,
+          snmp_community: @test_community,
+          snmp_port: 161
+        })
+
+      {:ok, snmp_device} = Discovery.discover_device(device)
+
+      # Verify SNMP device record has all fields
+      assert snmp_device.id
+      assert is_binary(snmp_device.sys_descr)
+      assert is_binary(snmp_device.sys_object_id)
+      assert is_binary(snmp_device.sys_name)
+      assert is_binary(snmp_device.manufacturer)
+      assert is_binary(snmp_device.model)
+      assert snmp_device.last_discovery_at
+
+      Logger.info(
+        "Discovery: manufacturer=#{snmp_device.manufacturer}, " <>
+          "model=#{snmp_device.model}, firmware=#{snmp_device.firmware_version}, " <>
+          "serial=#{snmp_device.serial_number}"
+      )
+
+      # Verify interfaces persisted and match raw walk count
+      interfaces = Snmp.list_interfaces(snmp_device.id)
+      {:ok, raw_if_indices} = Client.walk(@client_opts, @if_index_oid)
+
+      assert length(interfaces) == map_size(raw_if_indices),
+             "Persisted #{length(interfaces)} interfaces, raw walk has #{map_size(raw_if_indices)}"
+
+      Logger.info("Persisted #{length(interfaces)} interfaces (matches raw walk)")
+
+      # Verify sensors persisted (MikroTik should have profile-specific sensors)
+      sensors = Snmp.list_sensors(snmp_device.id)
+      assert is_list(sensors)
+
+      Logger.info("Persisted #{length(sensors)} sensors")
+
+      if sensors != [] do
+        by_type = Enum.group_by(sensors, & &1.sensor_type)
+
+        for {type, group} <- by_type do
+          Logger.info("  #{type}: #{length(group)} sensors")
+
+          for s <- group do
+            Logger.info("    #{s.sensor_descr} (#{s.sensor_oid})")
+          end
+        end
+      end
+
+      # Verify processors persisted
+      processors = Snmp.list_processors(snmp_device.id)
+      {:ok, raw_proc} = Client.walk(@client_opts, @hr_processor_load_oid)
+
+      assert length(processors) == map_size(raw_proc),
+             "Persisted #{length(processors)} processors, raw walk has #{map_size(raw_proc)}"
+
+      Logger.info("Persisted #{length(processors)} processors (matches raw walk)")
+
+      # Verify storage persisted (only disk-type entries, not RAM/Other)
+      storage = Snmp.list_storage(snmp_device.id)
+      assert is_list(storage)
+      assert storage != [], "Expected at least one disk storage entry"
+
+      Logger.info("Persisted #{length(storage)} disk storage entries")
+
+      # Verify IP addresses persisted
+      ip_addresses = Snmp.list_ip_addresses(snmp_device.id)
+      {:ok, raw_ips} = Client.walk(@client_opts, @ip_ad_ent_addr_oid)
+
+      Logger.info("Persisted #{length(ip_addresses)} IP addresses (raw walk has #{map_size(raw_ips)})")
+    end
+
+    @tag timeout: 300_000
+    test "rediscovery updates without creating duplicates", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      device =
+        device_fixture(%{
+          ip_address: @test_ip,
+          snmp_enabled: true,
+          snmp_version: @test_version,
+          snmp_community: @test_community,
+          snmp_port: 161
+        })
+
+      # First discovery
+      {:ok, snmp_device_1} = Discovery.discover_device(device)
+      interfaces_1 = Snmp.list_interfaces(snmp_device_1.id)
+      sensors_1 = Snmp.list_sensors(snmp_device_1.id)
+      processors_1 = Snmp.list_processors(snmp_device_1.id)
+      storage_1 = Snmp.list_storage(snmp_device_1.id)
+
+      # Second discovery (should update, not duplicate)
+      {:ok, snmp_device_2} = Discovery.discover_device(device)
+      interfaces_2 = Snmp.list_interfaces(snmp_device_2.id)
+      sensors_2 = Snmp.list_sensors(snmp_device_2.id)
+      processors_2 = Snmp.list_processors(snmp_device_2.id)
+      storage_2 = Snmp.list_storage(snmp_device_2.id)
+
+      # Same SNMP device record (upserted, not duplicated)
+      assert snmp_device_1.id == snmp_device_2.id
+
+      # Same counts (synced, not appended)
+      assert length(interfaces_1) == length(interfaces_2),
+             "Interface count changed: #{length(interfaces_1)} -> #{length(interfaces_2)}"
+
+      assert length(sensors_1) == length(sensors_2),
+             "Sensor count changed: #{length(sensors_1)} -> #{length(sensors_2)}"
+
+      assert length(processors_1) == length(processors_2),
+             "Processor count changed: #{length(processors_1)} -> #{length(processors_2)}"
+
+      assert length(storage_1) == length(storage_2),
+             "Storage count changed: #{length(storage_1)} -> #{length(storage_2)}"
+
+      Logger.info(
+        "Rediscovery preserved: #{length(interfaces_2)} interfaces, " <>
+          "#{length(sensors_2)} sensors, #{length(processors_2)} processors, " <>
+          "#{length(storage_2)} storage"
+      )
+    end
+  end
+
+  describe "polling after discovery" do
+    @tag timeout: 300_000
+    test "sensor OIDs return numeric values after discovery", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      device =
+        device_fixture(%{
+          ip_address: @test_ip,
+          snmp_enabled: true,
+          snmp_version: @test_version,
+          snmp_community: @test_community,
+          snmp_port: 161
+        })
+
+      {:ok, snmp_device} = Discovery.discover_device(device)
+      sensors = Snmp.list_sensors(snmp_device.id)
+
+      if Enum.empty?(sensors) do
+        Logger.warning("No sensors to poll - skipping sensor polling test")
+      else
+        # Poll each sensor's OID and verify we get a readable value
+        results =
+          Enum.map(sensors, fn sensor ->
+            case Client.get(@client_opts, sensor.sensor_oid) do
+              {:ok, value} when is_number(value) ->
+                {:numeric, sensor.sensor_descr, value}
+
+              {:ok, value} ->
+                {:non_numeric, sensor.sensor_descr, value}
+
+              {:error, reason} ->
+                {:error, sensor.sensor_descr, reason}
+            end
+          end)
+
+        numeric = Enum.count(results, &(elem(&1, 0) == :numeric))
+        non_numeric = Enum.count(results, &(elem(&1, 0) == :non_numeric))
+        errors = Enum.count(results, &(elem(&1, 0) == :error))
+
+        Logger.info(
+          "Polled #{length(sensors)} sensors: " <>
+            "#{numeric} numeric, #{non_numeric} non-numeric, #{errors} errors"
+        )
+
+        for {status, descr, val} <- results do
+          Logger.info("  [#{status}] #{descr}: #{inspect(val)}")
+        end
+
+        assert numeric > 0, "Expected at least one sensor to return numeric data"
+      end
+    end
+
+    @tag timeout: 120_000
+    test "interface counters (ifInOctets/ifOutOctets) are readable", %{reachable: reachable} do
+      skip_unless_reachable(reachable)
+
+      {:ok, in_octets} = Client.walk(@client_opts, @if_in_octets_oid)
+      {:ok, out_octets} = Client.walk(@client_opts, @if_out_octets_oid)
+
+      assert map_size(in_octets) > 0, "Expected ifInOctets data"
+      assert map_size(out_octets) > 0, "Expected ifOutOctets data"
+
+      # Counter values should be non-negative integers
+      for {oid, value} <- in_octets do
+        assert is_integer(value) and value >= 0,
+               "ifInOctets #{oid} invalid: #{inspect(value)}"
+      end
+
+      for {oid, value} <- out_octets do
+        assert is_integer(value) and value >= 0,
+               "ifOutOctets #{oid} invalid: #{inspect(value)}"
+      end
+
+      Logger.info("Interface counters: #{map_size(in_octets)} in, #{map_size(out_octets)} out")
+    end
+  end
+
+  # Helper to skip tests when device is unreachable.
+  defp skip_unless_reachable(true), do: :ok
+
+  defp skip_unless_reachable(false) do
+    flunk("Device #{@test_ip} unreachable - skipping")
+  end
+end

test/towerops/monitoring/check_test.exs

@@ -155,7 +155,7 @@ defmodule Towerops.Monitoring.CheckTest do
 
       changeset = Check.changeset(%Check{}, attrs)
       assert changeset.valid?
-      assert get_change(changeset, :response_time_ms) == 123.456
+      assert get_change(changeset, :response_time_ms) == 123.5
     end
 
     test "checked_at accepts DateTime", %{device: device} do