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

adding optional other sensor values #17

Open
cweinhofer opened this issue Jun 25, 2020 · 7 comments
Open

adding optional other sensor values #17

cweinhofer opened this issue Jun 25, 2020 · 7 comments

Comments

@cweinhofer
Copy link
Contributor

Hi György,

I like the recent changes to module. Both functional and attractive. Seeing them inspired me to rethink my pull request from a few months ago.

Your assertion about very few people ever needing to display both Celsius and Fahrenheit was quite true. But I realized that my my original proposal may not have been bad but simply too narrow in scope. I think the idea could be sound if it could be expanded for a wider variety of uses.

Here's my new idea: What if we added an option in the code to have two extra values displayed directly below temp and humidity and above your new battery and update time? They would use essentially the same code as temp and humidity with just new names.

And like humidity, they would only appear if data was sent to the module. For many users they would be unused -- "out of sight, out of mind" as the say. But for people who have remote units capable of sending other values (for example atmospheric pressure, wind speed, solar radiation level, etc.) they would be available for use.

To help keep things simple and broadly usable, they could be coded as just generic number values and the user would then add symbols/units (like atm or m/s) after the numbers using CSS.

I would be happy to cobble together a pull request for both the code and the readme. Code-wise the changes would be somewhat similar to my pull request from a few months ago, so I don't think it would be too hard to do. But I figured I'd see what you thought of the idea first.

Thanks,
Colby

@balassy
Copy link
Owner

balassy commented Aug 12, 2020

Hello Colby,

I apologize for the slow reply, I somehow lost the email notification.

Overall I like the idea to make the module more generic and allow the devices to submit more data that the module can display. Do you have any special data in mind that your devices could send and would like to see on your mirror? That would help us figure out the right data and configuration JSON formats.

For example the posted data could be something like this:

{
  "temp": 27,
  "humidity": 30.4,
  "battery": 100,
  "sensorId": "1",
  "ext": {
    "pressure": 1017,
    "windSpeed": 5
  }
}

And the corresponding configuration:

var config = {
  modules: [
    {
      module: 'MMM-RemoteTemperature',
      position: 'top_right',
      config: {
        sensorId: '1',
        icon: 'home',
        showMore: true,
        extUnits: {
          "pressure: "hPa",
          "windSpeed": "km/h"
        }
      }
    }
  ]
}

The above is just my first thought, I'm not sure that it is generic enough. For example I don't know if devices can send string data like cloudy and for that maybe we should handle all ext data as strings?

We can come up with a better name than ext (extension) too :)

We should also not forget that you mentioned that you have several devices you want to see on the same mirror. For that I think we should create an optional "condensed" view or just show these new values right after the current temperature and humidity values in the same line, and not add a second line above the "more" line.

What do you think?

@cweinhofer
Copy link
Contributor Author

No problem. Your timing is actually perfect because just this morning I was thinking about another value I'd like to display (heat index).

For the different types of data, I think a simpler approach would save you from having extra code to maintain and be more universal so people can use the two "ext" slots for whatever they want. (And I think "ext" is just fine.)

The posted data would be something like

{
  "temp": 27,
  "humidity": 30.4,
  "ext1": 1017,
  "ext2": 5,
  "battery": 100,
  "sensorId": "1"
}

People could then add their preferred labels in CSS like

.MMM-RemoteTemperature .ext1::after {
  content: "hPa";
  }
.MMM-RemoteTemperature .ext2::after {
  content: "km/h";
  }

I also don't think you need to worry about string values. Things like "cloudy" would require more calculation and interpretation than a simple sensor could handle, so I think pretty much any value coming from a sensor would be numeric.

For the formatting, I have found that more than two values on a single line is hard to look at. So, I'm guessing most people would want to have a line break between temp/humd and ext1/2. But other people may have different opinions. So maybe you could do the following:

  • The default format would be two line breaks, one between temp/humd and ext1/2, and one between ext1/2 and battery/time -- making three lines if all data is displayed.
  • Include a "noBreaks" option in the config which removes both line breaks. This would allow a person to have everything on one line or add line breaks of their choosing with CSS.

If you happen to have a testing branch I can access, I'm happy to do some beta testing of different format configurations.

@balassy
Copy link
Owner

balassy commented Aug 16, 2020

Hello @cweinhofer,

I created a new feat/extension-fields branch in this repository and a #19 PR that contains the changes. I wanted to make it both backward compatible and extensible, so now it is able to display as many data fields as the sensor can send. At the moment there are no noBreaks options, please let me know if you really need it.

I'm very interested in your opinion.

@cweinhofer
Copy link
Contributor Author

Thanks, György. Let me do some testing and I'll report back.

@cweinhofer
Copy link
Contributor Author

I've run some tests and here's what I've observed so far:

  1. At a basic level, the new code works just fine and I think most people would be quite happy. My observations below are just things that would allow deeper customization or better convenience for multiple instances.

  2. Having each ext unit as its own item under the ext property works great for extensibility. My only difficulty is that I don't like using the sub property entries to set the unit. In my 7-sensor setup I need to change something 7 times if I want to edit a unit. For my setup, adding the unit just one time in CSS would be more convenient. But I realize most users don't have 7 sensors like I do. However, If we can fix no 3 below, users would have the option to add the unit in config or omit it and add it in CSS.

  3. The CSS class for all ext values end up being just ext. If a user wants to style one ext value different from another (like adding units in CSS as in no 2 above), they can't do it. If it's not too much trouble, I would be great if each ext value got a separate class from its name in config.

  4. I don't personally have a problem with not having a noBreaks option. I find that three lines with two values each is the best compact format for me. But that's just me. Maybe others would feel differently.

  5. This isn't a problem, but just something we need to makes sure to put in the readme -- any units with special punctuation (like degree) can't be added as-is in the string in config. You have to enter them using HTML entity codes (like °).

  6. (this has nothing to do with the new code, I just noticed it when I was playing around with the CSS) The read me says the battery and update time have their own CSS classes -- battery and time, respectively. But the style inspector on my browser doesn't show any CSS class and when I try to style something with .MMM-RemoteTemperature .battery or .MMM-RemoteTemperature .time there is no change.

@cweinhofer
Copy link
Contributor Author

Doing some more playing around and have two further things to report. (nos. continued from above)

  1. No luck so far on adding the CSS class to the ext fields (no. 3 above). I was hoping it was as simple as changing extEl.classList = 'ext'; (line 78) to extEl.classList = `ext ${extData.value}`; but that didn't do anything. I was also thinking if we got the individual ext CSS classes working, I might look into adding icons before those fields using ::before in CSS. If you have any thoughts or tips feel free to pass them along.

  2. I've discovered if your sensor sends a value of 0 to the module for a specific field, the module display that field as blank. I'm guessing you already know that, but it will be another helpful thing to add to the readme. It ends up being useful for instances where you want to use the same code for multiple sensors, but only some of them have data for a specific field.

@cweinhofer
Copy link
Contributor Author

@balassy

Hi György,

I wanted to check on this to see where things stand? I have been using the feat/extension-fields for a while now and it seems to work fine. I even found another use for the ext fields recently. I started having the microcontroller in my temperature sensors keep a record of the high and low temperatures for each day as well as calculate the heat index using the temperature and humidity. Using the ext fields I can display this below my main temp readout.

Screenshot 2021-05-13 174610

However, I'm still running into the problem I mentioned in no. 3, above. Because the ext fields don't have their own CSS class, I can't style them individually. In my case I'd like to ad the label (H, L, or I for high, low, index) before each temp ussing CSS rather than after using the field in the module.

Also, another smaller issue I bumped into:

  1. I'm using transform: scale in my CSS to make the ext fields smaller so the main temp is more prominent. But when I do this, only the characters get scaled. The space between the parts remains the same, which make things look kind of strung out and takes up unneeded space. Any idea how to reduce this space?

Thanks!

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

No branches or pull requests

2 participants