From 6e9ff55a3899d5f0ab43cb9c5951ac16e6d62bbf Mon Sep 17 00:00:00 2001 From: Rick Carlino Date: Thu, 5 Sep 2019 17:27:42 -0500 Subject: [PATCH] Reducer is now tracking QoS stats. NEXT: Add to UI, write tests. --- frontend/__tests__/interceptors_test.ts | 6 ++++-- .../__tests__/connect_device/index_test.ts | 18 +++++++++------- .../connectivity/__tests__/ping_mqtt_test.ts | 8 +++---- frontend/connectivity/connect_device.ts | 11 +++++----- frontend/connectivity/index.ts | 8 +++---- frontend/connectivity/log_handlers.ts | 3 ++- frontend/connectivity/ping_mqtt.tsx | 21 +++++++++---------- frontend/connectivity/reducer.ts | 2 -- frontend/devices/connectivity/qos.ts | 2 +- frontend/interceptors.ts | 7 ++++--- 10 files changed, 46 insertions(+), 40 deletions(-) diff --git a/frontend/__tests__/interceptors_test.ts b/frontend/__tests__/interceptors_test.ts index d1a8995d0..7316ac461 100644 --- a/frontend/__tests__/interceptors_test.ts +++ b/frontend/__tests__/interceptors_test.ts @@ -31,6 +31,8 @@ import { dispatchNetworkUp, dispatchNetworkDown } from "../connectivity"; import { Session } from "../session"; import { error } from "../toast/toast"; +const ANY_NUMBER = expect.any(Number); + interface FakeProps { uuid: string; method: Method; @@ -65,7 +67,7 @@ describe("responseRejected", () => { it("undefined error", async () => { await expect(responseRejected(undefined)).rejects.toEqual(undefined); expect(dispatchNetworkUp).not.toHaveBeenCalled(); - expect(dispatchNetworkDown).toHaveBeenCalledWith("user.api"); + expect(dispatchNetworkDown).toHaveBeenCalledWith("user.api", ANY_NUMBER); }); it("safe error", async () => { @@ -75,7 +77,7 @@ describe("responseRejected", () => { }; await expect(responseRejected(safeError)).rejects.toEqual(safeError); expect(dispatchNetworkDown).not.toHaveBeenCalled(); - expect(dispatchNetworkUp).toHaveBeenCalledWith("user.api"); + expect(dispatchNetworkUp).toHaveBeenCalledWith("user.api", ANY_NUMBER); }); it("handles 500", async () => { diff --git a/frontend/connectivity/__tests__/connect_device/index_test.ts b/frontend/connectivity/__tests__/connect_device/index_test.ts index cab29d82c..16a1ce74a 100644 --- a/frontend/connectivity/__tests__/connect_device/index_test.ts +++ b/frontend/connectivity/__tests__/connect_device/index_test.ts @@ -42,6 +42,8 @@ import { MessageType } from "../../../sequences/interfaces"; import { FbjsEventName } from "farmbot/dist/constants"; import { info, error, success, warning, fun, busy } from "../../../toast/toast"; +const ANY_NUMBER = expect.any(Number); + describe("readStatus()", () => { it("forces a read_status request to FarmBot", () => { readStatus(); @@ -160,8 +162,8 @@ describe("initLog", () => { describe("bothUp()", () => { it("marks MQTT and API as up", () => { bothUp(); - expect(dispatchNetworkUp).toHaveBeenCalledWith("user.mqtt"); - expect(dispatchNetworkUp).toHaveBeenCalledWith("bot.mqtt"); + expect(dispatchNetworkUp).toHaveBeenCalledWith("user.mqtt", ANY_NUMBER); + expect(dispatchNetworkUp).toHaveBeenCalledWith("bot.mqtt", ANY_NUMBER); }); }); @@ -169,7 +171,7 @@ describe("onOffline", () => { it("tells the app MQTT is down", () => { jest.resetAllMocks(); onOffline(); - expect(dispatchNetworkDown).toHaveBeenCalledWith("user.mqtt"); + expect(dispatchNetworkDown).toHaveBeenCalledWith("user.mqtt", ANY_NUMBER); expect(error).toHaveBeenCalledWith(Content.MQTT_DISCONNECTED); }); }); @@ -178,7 +180,7 @@ describe("onOnline", () => { it("tells the app MQTT is up", () => { jest.resetAllMocks(); onOnline(); - expect(dispatchNetworkUp).toHaveBeenCalledWith("user.mqtt"); + expect(dispatchNetworkUp).toHaveBeenCalledWith("user.mqtt", ANY_NUMBER); }); }); @@ -204,13 +206,14 @@ describe("onSent", () => { it("marks MQTT as up", () => { jest.resetAllMocks(); onSent({ connected: true })(); - expect(dispatchNetworkUp).toHaveBeenCalledWith("user.mqtt"); + expect(dispatchNetworkUp).toHaveBeenCalledWith("user.mqtt", ANY_NUMBER); }); it("marks MQTT as down", () => { jest.resetAllMocks(); onSent({ connected: false })(); - expect(dispatchNetworkDown).toHaveBeenCalledWith("user.mqtt"); + expect(dispatchNetworkDown) + .toHaveBeenCalledWith("user.mqtt", ANY_NUMBER); }); }); @@ -234,7 +237,8 @@ describe("onLogs", () => { log.message = "bot xyz is offline"; fn(log); globalQueue.maybeWork(); - expect(dispatchNetworkDown).toHaveBeenCalledWith("bot.mqtt"); + expect(dispatchNetworkDown) + .toHaveBeenCalledWith("bot.mqtt", ANY_NUMBER); }); it("handles log fields correctly", () => { diff --git a/frontend/connectivity/__tests__/ping_mqtt_test.ts b/frontend/connectivity/__tests__/ping_mqtt_test.ts index f3b69eb79..e743cf6d6 100644 --- a/frontend/connectivity/__tests__/ping_mqtt_test.ts +++ b/frontend/connectivity/__tests__/ping_mqtt_test.ts @@ -3,7 +3,7 @@ jest.mock("../index", () => ({ dispatchNetworkUp: jest.fn(), dispatchQosStart: jest.fn() })); - +const ANY_NUMBER = expect.any(Number); const mockTimestamp = 0; jest.mock("../../util", () => ({ timestamp: () => mockTimestamp })); @@ -50,14 +50,14 @@ function fakeBot(): Farmbot { function expectStale() { expect(dispatchNetworkDown) - .toHaveBeenCalledWith("bot.mqtt"); + .toHaveBeenCalledWith("bot.mqtt", ANY_NUMBER, "TESTS"); } function expectActive() { expect(dispatchNetworkUp) - .toHaveBeenCalledWith("bot.mqtt"); + .toHaveBeenCalledWith("bot.mqtt", ANY_NUMBER, "TESTS"); expect(dispatchNetworkUp) - .toHaveBeenCalledWith("user.mqtt"); + .toHaveBeenCalledWith("user.mqtt", ANY_NUMBER, "TESTS"); } describe("ping util", () => { diff --git a/frontend/connectivity/connect_device.ts b/frontend/connectivity/connect_device.ts index e7f802e2b..b0e7b5297 100644 --- a/frontend/connectivity/connect_device.ts +++ b/frontend/connectivity/connect_device.ts @@ -28,6 +28,7 @@ import { ChannelName, MessageType } from "../sequences/interfaces"; import { DeepPartial } from "redux"; import { slowDown } from "./slow_down"; import { t } from "../i18next_wrapper"; +import { now } from "../devices/connectivity/qos"; export const TITLE = () => t("New message from bot"); /** TODO: This ought to be stored in Redux. It is here because of historical @@ -96,8 +97,8 @@ export const batchInitResources = }; export const bothUp = () => { - dispatchNetworkUp("user.mqtt"); - dispatchNetworkUp("bot.mqtt"); + dispatchNetworkUp("user.mqtt", now()); + dispatchNetworkUp("bot.mqtt", now()); }; export function readStatus() { @@ -108,7 +109,7 @@ export function readStatus() { } export const onOffline = () => { - dispatchNetworkDown("user.mqtt"); + dispatchNetworkDown("user.mqtt", now()); error(t(Content.MQTT_DISCONNECTED)); }; @@ -151,7 +152,7 @@ type Client = { connected?: boolean }; export const onSent = (client: Client) => () => { const connected = !!client.connected; const cb = connected ? dispatchNetworkUp : dispatchNetworkDown; - cb("user.mqtt"); + cb("user.mqtt", now()); }; export function onMalformed() { @@ -165,7 +166,7 @@ export function onMalformed() { export const onOnline = () => { success(t("Reconnected to the message broker."), t("Online")); - dispatchNetworkUp("user.mqtt"); + dispatchNetworkUp("user.mqtt", now()); }; export const onReconnect = () => warning(t("Attempting to reconnect to the message broker"), diff --git a/frontend/connectivity/index.ts b/frontend/connectivity/index.ts index f037c62b5..c0559a902 100644 --- a/frontend/connectivity/index.ts +++ b/frontend/connectivity/index.ts @@ -32,16 +32,16 @@ export const dispatchQosStart = (id: string) => { }); }; -export let dispatchNetworkUp = (edge: Edge, at = (new Date()).getTime()) => { +export let dispatchNetworkUp = (edge: Edge, at: number, qosPingId?: string) => { console.log("TODO: Insert ID HERE"); if (shouldThrottle(edge, at)) { return; } - store.dispatch(networkUp(edge, at)); + store.dispatch(networkUp(edge, at, qosPingId)); bumpThrottle(edge, at); }; -export let dispatchNetworkDown = (edge: Edge, at = (new Date()).getTime()) => { +export let dispatchNetworkDown = (edge: Edge, at: number, qosPingId?: string) => { console.log("TODO: Insert ID HERE"); if (shouldThrottle(edge, at)) { return; } - store.dispatch(networkDown(edge, at)); + store.dispatch(networkDown(edge, at, qosPingId)); bumpThrottle(edge, at); }; diff --git a/frontend/connectivity/log_handlers.ts b/frontend/connectivity/log_handlers.ts index 6741f0383..98051e60b 100644 --- a/frontend/connectivity/log_handlers.ts +++ b/frontend/connectivity/log_handlers.ts @@ -11,6 +11,7 @@ import { Log } from "farmbot/dist/resources/api_resources"; import { globalQueue } from "./batch_queue"; import { isUndefined, get } from "lodash"; import { MessageType } from "../sequences/interfaces"; +import { now } from "../devices/connectivity/qos"; const LEGACY_META_KEY_NAMES: (keyof Log)[] = [ "type", @@ -49,6 +50,6 @@ export const onLogs = // TODO: Make a `bot/device_123/offline` channel. const died = msg.message.includes("is offline") && msg.type === MessageType.error; - died && dispatchNetworkDown("bot.mqtt"); + died && dispatchNetworkDown("bot.mqtt", now()); } }; diff --git a/frontend/connectivity/ping_mqtt.tsx b/frontend/connectivity/ping_mqtt.tsx index 682bddc4e..c1013a7e2 100644 --- a/frontend/connectivity/ping_mqtt.tsx +++ b/frontend/connectivity/ping_mqtt.tsx @@ -8,6 +8,7 @@ import { isNumber } from "lodash"; import axios from "axios"; import { API } from "../api/index"; import { FarmBotInternalConfig } from "farmbot/dist/config"; +import { now } from "../devices/connectivity/qos"; export const PING_INTERVAL = 4000; export const ACTIVE_THRESHOLD = PING_INTERVAL * 2; @@ -21,19 +22,17 @@ export function readPing(bot: Farmbot, direction: Direction): number | undefined return isNumber(val) ? val : undefined; } -export function markStale(_uuid: string) { - // dispatch({ pings: failPing(this.pingState, id) }) - dispatchNetworkDown("bot.mqtt"); +export function markStale(qosPingId: string) { + dispatchNetworkDown("bot.mqtt", now(), qosPingId); } -export function markActive(_uuid: string) { - // dispatch({ pings: completePing(this.pingState, id) }) - dispatchNetworkUp("user.mqtt"); - dispatchNetworkUp("bot.mqtt"); +export function markActive(qosPingId: string) { + dispatchNetworkUp("user.mqtt", now(), qosPingId); + dispatchNetworkUp("bot.mqtt", now(), qosPingId); } -export function isInactive(last: number, now: number): boolean { - return last ? (now - last) > ACTIVE_THRESHOLD : true; +export function isInactive(last: number, now_: number): boolean { + return last ? (now_ - last) > ACTIVE_THRESHOLD : true; } export function sendOutboundPing(bot: Farmbot) { @@ -51,7 +50,7 @@ export function startPinging(bot: Farmbot) { } export function pingAPI() { - const ok = () => dispatchNetworkUp("user.api"); - const no = () => dispatchNetworkDown("user.api"); + const ok = () => dispatchNetworkUp("user.api", now()); + const no = () => dispatchNetworkDown("user.api", now()); return axios.get(API.current.devicePath).then(ok, no); } diff --git a/frontend/connectivity/reducer.ts b/frontend/connectivity/reducer.ts index 360673fe3..869375710 100644 --- a/frontend/connectivity/reducer.ts +++ b/frontend/connectivity/reducer.ts @@ -30,10 +30,8 @@ export let connectivityReducer = const { qosPingId, status } = payload; if (qosPingId) { if (status.state == "up") { - console.log("OK!!!"); s.pings = completePing(s.pings, qosPingId, status.at); } else { - console.log("FAILED"); s.pings = failPing(s.pings, qosPingId); } } diff --git a/frontend/devices/connectivity/qos.ts b/frontend/devices/connectivity/qos.ts index 347a1563c..a2e0f5804 100644 --- a/frontend/devices/connectivity/qos.ts +++ b/frontend/devices/connectivity/qos.ts @@ -24,7 +24,7 @@ interface Complete { export type Ping = Complete | Pending | Timeout; export type PingDictionary = Record; -const now = () => (new Date()).getTime(); +export const now = () => (new Date()).getTime(); export const startPing = (s: PingDictionary, id: string, start = now()): PingDictionary => { diff --git a/frontend/interceptors.ts b/frontend/interceptors.ts index b2b12d157..4e2a09021 100644 --- a/frontend/interceptors.ts +++ b/frontend/interceptors.ts @@ -10,9 +10,10 @@ import { Session } from "./session"; import { get } from "lodash"; import { t } from "./i18next_wrapper"; import { error } from "./toast/toast"; +import { now } from "./devices/connectivity/qos"; export function responseFulfilled(input: AxiosResponse): AxiosResponse { - dispatchNetworkUp("user.api"); + dispatchNetworkUp("user.api", now()); return input; } @@ -26,7 +27,7 @@ export const isLocalRequest = (x: SafeError) => let ONLY_ONCE = true; export function responseRejected(x: SafeError | undefined) { if (x && isSafeError(x)) { - dispatchNetworkUp("user.api"); + dispatchNetworkUp("user.api", now()); const a = ![451, 401, 422].includes(x.response.status); const b = x.response.status > 399; // Openfarm API was sending too many 404's. @@ -57,7 +58,7 @@ export function responseRejected(x: SafeError | undefined) { } return Promise.reject(x); } else { - dispatchNetworkDown("user.api"); + dispatchNetworkDown("user.api", now()); return Promise.reject(x); } }