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

Improve display separation #20495

Closed
wants to merge 3 commits into from
Closed

Improve display separation #20495

wants to merge 3 commits into from

Conversation

SteWers
Copy link
Contributor

@SteWers SteWers commented Jan 14, 2024

@arendst I want you to ask you for your opinion to this PR.

My intention for this PR was that there is a not so good separation of values from different sensors at the moment. Here an example:
image

It seems that the ESP32 temperature belongs to the sensor output above. This is most noticeable, when a sensor reports several values. To get a better readable display, I searched for as good and simple solution.

The idea was to insert a separation line at the beginning of a sensor output. To avoid a separation line in front of the first sensor is displayed, I use the new variable TasmotaGlobal.FirstLineSend. (Is this place suitable?)
It is set to false before XsnsXdrvCall(FUNC_WEB_SENSOR) is called. When something is send to the browser it is set to true. Therefore, every driver can decide to use this variable and insert a separation line, if this is makes sense. This could look like this:
if (TasmotaGlobal.FirstLineSend) WSContentSend_P("<tr><td colspan=2><hr>{e}");
This will generate an uninterrupted line between different sensors. To separate different “blocks” form one sensor, the established interrupted line could be used.

I tried this with a few drivers, as you can see in the PR. It looks very good in my opinion.
image

To save some bytes the line definitions could be stored in global consts, so that they have not defined separately in each driver. E.g.:

const char HTTP_SENSOR_SEPARATOR[]      PROGMEM = "<tr><td colspan=2><hr>{e}";
const char HTTP_VALUE_SEPARATOR[]       PROGMEM = "{s}<hr>{m}<hr>{e}";
const char HTTP_VALUE_SEPARATOR_THIN[]  PROGMEM = "{s}<hr size=1>{m}<hr size=1>{e}";

Where is the best place for this?

Please take a look at my idea and I look forward to your opinion and suggestions.

P.S.: In xsns_62_esp32_mi_ble.ino are several codecosmetical changes, which are not directly related to the intention of the PR.

@barbudor
Copy link
Contributor

If every driver needs to be updated for that , I don't see that a viable solution
If it has to be done, I would assume it has to be done by Tasmota core so every sensor group would be separated

@SteWers
Copy link
Contributor Author

SteWers commented Jan 14, 2024

@barbudor Sorry, I didn't wrote it. No, there is no change necessary to any driver. If nothing is changed in the driver, the behavior of the display for the driver stays as before.

@arendst
Copy link
Owner

arendst commented Jan 14, 2024

The gui was never to become a book of sensors. I saw horrific ble and zigbee lists. Adding a separation between the different sensors makes the list even longer (sigh).

If separation is really needed then the barbudor solution is the most code cost effective solution. I'll see how it looks if inserted between every sensors gui update call.

@barbudor
Copy link
Contributor

Therefore, every driver can decide to use this variable and insert a separation line, if this is makes sense

HI @SteWers
May be I mis-understood that sentence

Therefore, every driver can decide to use this variable and insert a separation line, if this is makes sense

I understood it as every driver would have to check the variable and insert the line

@SteWers
Copy link
Contributor Author

SteWers commented Jan 14, 2024

Yes, of course it makes long lists longer. But in long lists the separation is much more important than in short lists. With this PR every sensor can "decide" of a separation or not. A core solution would separate all sensors, which is not always necessary.
Beside of that I would prefer a core solution.
@barbudor The magic word is can not must. 😊 Only if you want a separation line, you have to insert a line like in the sample.

@arendst
Copy link
Owner

arendst commented Jan 15, 2024

I must say I like it. Even on single sensors:

image

My solution leans on @SteWers but it is less intrusive. I feel another SetOption is coming along....

void WSSeparator(uint32_t state) {
  static bool first = true;
  static bool request = false;
  switch (state) {
    case 0:   // Reset
      first = true;
      request = false;
      break;
    case 1:   // Print separator if needed
      if (first) {
        first = false;
      } else {
        if (request) {
//          WSContentSend_P(PSTR("{s}<hr>{m}<hr>{e}"));    // {s} = <tr><th>, {m} = </th><td>, {e} = </td></tr>
          WSContentSend_P("<tr><td colspan=2><hr>{e}");
          request = false;
        }
      }
      break;
    case 2:  // Request 
      request = true;
      break;
  }
}

@SteWers
Copy link
Contributor Author

SteWers commented Jan 15, 2024

Nice to hear. I worked a bit on the core solution. Will update this PR later today. I thought about a SetOption too.

@Jason2866
Copy link
Collaborator

Agree, looks nicer with lines.

@arendst
Copy link
Owner

arendst commented Jan 15, 2024

Agree, looks nicer with lines.

That's it. I make it permanent. Need to remove <hr> from some other drivers saving some bytes too ;-)

@SteWers
Copy link
Contributor Author

SteWers commented Jan 15, 2024

Need to remove <hr> from some other drivers

I think you mean energy for instance. 😉 I found some other drivers to be modified. You may have a look at my last changes.

arendst added a commit that referenced this pull request Jan 15, 2024
@arendst
Copy link
Owner

arendst commented Jan 15, 2024

OK. I've added separators to the GUI.

Some drivers probably still need to have changes but I leave it to the owners to make PR's as they like.

NB. Open item I still need to find a solution for is changing the height to a smaller value

@SteWers
Copy link
Contributor Author

SteWers commented Jan 15, 2024

OK. I've added separators to the GUI.

Thanks!

Some drivers probably still need to have changes but I leave it to the owners to make PR's as they like.

I will do a new PR with the sensor I've modified.

NB. Open item I still need to find a solution for is changing the height to a smaller value

Let me think about it. Maybe I've an idea...

@SteWers SteWers closed this Jan 15, 2024
@SteWers SteWers deleted the HR branch January 19, 2024 07:24
hawa-lc4 pushed a commit to hawa-lc4/Tasmota-dev that referenced this pull request Jan 20, 2024
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