Gladys V4 - Sensor display in the interface

Presentations
1

Hello everyone,

I wanted to post a message following an issue I have with the xiaomi module.

Indeed, when you retrieve a sensor, it will be added and visible in the view, for example « device ». The problem is the naming of these sensors. In short, the name must be unique. However, it is silly to have a unique name when in each room the sensor can very well be called « temperature sensor ». Except that currently it is a unique name, so it becomes machine room sensor or sensor 1 …

Wouldn’t it be more interesting to implement automatic naming that takes into account the type of sensor and displays specific naming? That way, on the dev side, we just need to add the device and put the name we want, and the front end handles the naming on its own.

What do you think?

2

Hello,
I also find the unique name very limiting, as I have several temperature sensors.
This name should be user-modifiable and should only serve as a label, not as a functional value.
Now that it is generated automatically, I think it might result in ridiculous names.

3

No, I think you haven’t fully understood.

Logically, in your room, you have 1 temperature sensor. Given that it is in this room, it can be named « Temperature Sensor ». This name can be present multiple times as it is unique to each room.

If you ever have two sensors, you name them 1 - 2.

The generation is done on the fly and will not be logically stored in the database.

The second idea is to define a new variable that is not the name and is not unique. This will allow you to have multiple « Temperature Sensors » with the same name if the user wishes.

In essence, you look at other systems, you define no name, you just add equipment to a room.

4

@damalgos: I’m going to remove the uniqueness constraint on the name, it doesn’t make sense :slight_smile: Anyway, the way we started developing the services in Gladys 4, the « name » is useless and not used :slight_smile:

5

Oh, that’s some good news :smiley:

6

Oh well, I realized that SQLite doesn’t allow removing unique indexes during a migration, so it’s not as simple as that ^^

If we had been in a production release, we would have had to:

  1. Temporarily disable foreign keys for the migration
  2. Rename the table t_device to t_device_old
  3. Create the new table t_device
  4. Transfer data from the old to the new table
  5. Delete the old table
  6. Re-enable foreign keys

Given that we’re still in alpha and there’s no sensitive user data yet, the simplest approach for the developer (to save time) is to modify the initial migrations directly, which means that everyone who has installed the alpha will lose their data and have to recreate the DB!

I’ll take this opportunity to review the entire DB to see if there are any other unnecessary constraints to remove. I noticed that the calendar and events have the same issue, so I’ll remove those as well.

Not very fun, but hey, it’s an alpha. Of course, this kind of procedure will never be done once we’re in production.

7

Hello @Pierre-Gilles, I had the same difficulty as you in removing the uniqueness of the database, but it’s possible in the end. Check out the CalDAV PR: https://github.com/GladysAssistant/Gladys/pull/507/commits/24e95947bbad98c23856e7a36491102916b6c054#diff-875173103ade96e5af1bdede5b6ae683R3

8

Are you sure it works? According to my research and tests, SQLite does not allow removing a uniqueness constraint; you need to go through table creation + migration. Maybe I’m wrong, let me know :slight_smile:

9

Yes, it works very well, I had a problem with the uniqueness of names for recurring events, with this we can now save as many events Pepper's Birthday as we want.

10

Okay, thanks for your feedback! I’ll test that :slight_smile:

11

@bertrandda Unfortunately, your solution doesn’t work 100%. In fact, you’ve pointed out a bug (or a feature^^) in Sequelize!

In SQLite, ALTER TABLE doesn’t exist. The only way to update a table is to rename it, recreate it, and migrate the data.

Sequelize has abstracted this part and does it when you perform a changeColumn:

Executing (default): PRAGMA TABLE_INFO(`t_device`);
Executing (default): CREATE TABLE IF NOT EXISTS `t_device_backup` (`id` UUID NOT NULL PRIMARY KEY, `service_id` UUID NOT NULL, `room_id` UUID, `name` VARCHAR(255) NOT NULL, `selector` VARCHAR(255) NOT NULL, `model` VARCHAR(255), `external_id` VARCHAR(255) NOT NULL, `should_poll` TINYINT(1) NOT NULL DEFAULT 0, `poll_frequency` INTEGER, `created_at` DATETIME NOT NULL, `updated_at` DATETIME NOT NULL);
Executing (default): INSERT INTO `t_device_backup` SELECT `id`, `service_id`, `room_id`, `name`, `selector`, `model`, `external_id`, `should_poll`, `poll_frequency`, `created_at`, `updated_at` FROM `t_device`;
Executing (default): DROP TABLE `t_device`;
Executing (default): CREATE TABLE IF NOT EXISTS `t_device` (`id` UUID NOT NULL PRIMARY KEY, `service_id` UUID NOT NULL, `room_id` UUID, `name` VARCHAR(255) NOT NULL, `selector` VARCHAR(255) NOT NULL, `model` VARCHAR(255), `external_id` VARCHAR(255) NOT NULL, `should_poll` TINYINT(1) NOT NULL DEFAULT 0, `poll_frequency` INTEGER, `created_at` DATETIME NOT NULL, `updated_at` DATETIME NOT NULL);
Executing (default): INSERT INTO `t_device` SELECT `id`, `service_id`, `room_id`, `name`, `selector`, `model`, `external_id`, `should_poll`, `poll_frequency`, `created_at`, `updated_at` FROM `t_device_backup`;
Executing (default): DROP TABLE `t_device_backup`;

Or, if you read the code carefully, there is a bug in this implementation: foreign keys and constraints are not migrated. So yes, your name no longer has constraints: but all other columns have also lost their constraints, and the table ends up with no protections. The selector is no longer unique, the foreign keys are gone, etc…

See code:

After digging through Sequelize’s GitHub, there is an issue created in 2017 that references the problem:
https://github.com/sequelize/sequelize/issues/7078

But apparently, the bug is still not resolved..

Three options:

  • We use a « hack » that considers overriding one of Sequelize’s functions during this migration to re-add the constraints.
  • We find a way to fix Sequelize and propose a PR (though I’m not sure it will be merged before we need it)
  • We forget about doing a changeColumn and ask people to recreate the DB… which fixes it in the short term, but if in the long term we want to change a column, the problem is still there!
12

I managed to do the hack, here’s what it looks like:

module.exports = {
  up: async (queryInterface, Sequelize) => {
    const backupDescribeTable = queryInterface.describeTable;
    queryInterface.describeTable = async (tableName, options) => {
      const tableDatas = await backupDescribeTable.call(queryInterface, tableName, options);
      // get foreign key info
      const foreignKeyDatas = (await queryInterface.sequelize.query(`PRAGMA FOREIGN_KEY_LIST(\"${tableName}\");`))[0];
      foreignKeyDatas.forEach((foreignKeyData) => {
        const tableData = tableDatas[foreignKeyData.from];
        tableData.references = { model: foreignKeyData.table, key: foreignKeyData.to };
        tableData.onDelete = foreignKeyData.on_delete;
        tableData.onUpdate = foreignKeyData.on_update;
      });
      if (tableName === 't_device') {
        tableDatas.selector.unique = true;
        tableDatas.external_id.unique = true;
      }
      return tableDatas;
    };
    await queryInterface.changeColumn('t_device', 'name', {
      allowNull: false,
      type: Sequelize.STRING,
      unique: false,
    });
    queryInterface.describeTable = backupDescribeTable;
  },

  down: async (queryInterface, Sequelize) => {
    const backupDescribeTable = queryInterface.describeTable;
    queryInterface.describeTable = async (tableName, options) => {
      const tableDatas = await backupDescribeTable.call(queryInterface, tableName, options);
      // get foreign key info
      const foreignKeyDatas = (await queryInterface.sequelize.query(`PRAGMA FOREIGN_KEY_LIST(\"${tableName}\");`))[0];
      foreignKeyDatas.forEach((foreignKeyData) => {
        const tableData = tableDatas[foreignKeyData.from];
        tableData.references = { model: foreignKeyData.table, key: foreignKeyData.to };
        tableData.onDelete = foreignKeyData.on_delete;
        tableData.onUpdate = foreignKeyData.on_update;
      });
      if (tableName === 't_device') {
        tableDatas.name.unique = true;
        tableDatas.selector.unique = true;
        tableDatas.external_id.unique = true;
      }
      return tableDatas;
    };
    await queryInterface.changeColumn('t_device', 'name', {
      allowNull: false,
      type: Sequelize.STRING,
      unique: true,
    });
    queryInterface.describeTable = backupDescribeTable;
  },
};
13

Well, it’s not as simple as that… It’s even worse than I thought ^^

With « ON DELETE CASCADE », when we migrate the t_device table (deletion + recreation), it removes all entries from the t_device_feature table and consequently from t_device_feature_state… Not simple!

The only solution I see to our problem is either:

  • We do a mega custom special SQLite for our migrations… By writing the migrations ourselves in SQL, and we lose the idea of one day letting users use MySQL. Or we have two cases: one for MySQL, and one for SQLite (possible, but it’s work).

  • We go back to the original idea, since it’s an alpha we ask users to recreate the DB, and we modify the migrations.

I honestly think, given the scale it’s taking, we have every interest in modifying the original migrations (since we’re still « in dev », the product is not out) and to ask the question when we really have no choice (i.e., if we ever really have a column to modify). Knowing that this rarely happens (it really has to be a design flaw, like here). Usually, the most frequent is the addition of a column, and SQLite allows that.

14

We’re still in development, or recreating everything, which will also allow us to re-test the services and note the « imperfections ».