Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add/unify decentlab devices #76

Merged
merged 6 commits into from
Aug 5, 2024
Merged

Add/unify decentlab devices #76

merged 6 commits into from
Aug 5, 2024

Conversation

decentlab
Copy link
Contributor

@decentlab decentlab commented Aug 1, 2024

Decoder Description

Added all Decentlab device (connector) decoders from TTN repository.

Type of change

  • Adding a new Decoder of Connector
  • Update or fixing an issue in existing Decoder

Decoder Information and Payload to test and review

Checklist for Adding a New Decoder

  • Added a network.jsonc or connector.jsonc file that follows the structure defined in ./schema/.
  • Created a new folder under ./decoders/network/ or ./decoders/connector/ with the name of your decoder.
  • Created version folders and added manifest.jsonc files for each version.
  • Followed the folder structure guidelines for manufacturer and sensor/device model.
  • The code has unit test and it's in TypeScript.

Additional Notes

The existing decoders have been reverted back to the original TTN device repo version. This simplifies creating the decoders and makes sure that they uniformly work as expected over many platforms. Besides, the modifications made by TagoIO was mainly cosmetic, and there was no logic change.

We intentionally keep the decoders' syntax such that they're supported on most platforms with a minimal change.

@mateuscardosodeveloper
Copy link
Collaborator

Hello @decentlab, thank you for opening a Pull Request to add a public decoder. We appreciate your effort and contribution to the TagoIO IoT community.

We will review your PR as soon as possible.

Copy link
Collaborator

@mateuscardosodeveloper mateuscardosodeveloper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few points that need to be corrected or improved:

  1. The images should have a transparent background.
  2. The description.md file serves as the subtitle displayed on the devices' screen.
    a. This field is searchable. (When a user searches for a device on the platform, they use these words).
    b. It is recommended that description.md be clear and focus on the specific capabilities and features of the device.
  3. The new decoders added must be made in TypeScript and also add type annotation. E.g:
    a. When changing the payload to TypeScript, don't forget to edit the connector.jsonc file to payload.ts.
function toTagoFormat(object_item: {}, group: number, prefix: string = "") {
  //...
}
  1. Each decoder must have unit tests. Here is an example of a unit test you can use.
    a. If you can add a unit test for the payload parser, that would be great.
import { describe, test, expect } from "vitest";

import { decoderRun } from "../../../../../src/functions/decoder-run";

const file_path = "decoders/connector/your-path/payload.js" as const;

describe("Shall not be parsed", () => {
  let payload = [{ variable: "shallnotpass", value: "04096113950292" }];
  payload = decoderRun(file_path, { payload });
  test("Output Result", () => {
    expect(Array.isArray(payload)).toBe(true);
  });

  test("Not parsed Result", () => {
    expect(payload).toEqual([{ variable: "shallnotpass", value: "04096113950292" }]);
  });
});

@decentlab
Copy link
Contributor Author

decentlab commented Aug 1, 2024

Thanks for the review.

We followed the existing payload decoders that were added by TagoIO. It would have been more efficient if we knew the different requirements before creating these decoders. Would it be possible to add the decoders now as it is, and we improve them iteratively?

Some questions:

  • 2a, 2b: Could you make an example improvement on one device please?
  • 3a: Do you mean that we need to restructure all payload decoders to a real TypeScript? Or just add type annotations in the function signatures? Or change just the toTagoFormat signature?

@mateuscardosodeveloper
Copy link
Collaborator

Thanks for the review.

We followed the existing payload decoders that were added by TagoIO. It would have been more efficient if we knew the different requirements before creating these decoders. Would it be possible to add the decoders now as it is, and we improve them iteratively?

Some questions:

  • 2a, 2b: Could you make an example improvement on one device please?
  • 3a: Do you mean that we need to restructure all payload decoders to a real TypeScript? Or just add type annotations in the function signatures? Or change just the toTagoFormat signature?

Since the payload was created by TagoIO, I see no issue in approving it as it currently stands. However, I kindly request that you add the unit test mentioned in topic 4.

Regarding points 2a and 2b: These are just informational, indicating that they pertain to the searchable fields within the platform for the sensor.
searchable-devices

@decentlab
Copy link
Contributor Author

Thanks for considering to approve the decoders as they are now. We have added unit tests for the payload parsers.

Copy link
Collaborator

@Freddyminu Freddyminu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the sensor name from all description.md files.

Additionally, ensure that the install_text contains only factual information about the device, avoiding any subjective phrases.

@decentlab
Copy link
Contributor Author

@Freddyminu, these have been done by TagoIO and we followed exactly how it was. Would it be possible if you approve this PR now, and we improve iteratively? Thanks.

@FabianoEger FabianoEger merged commit 6ef5514 into tago-io:main Aug 5, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants