Skip to content

add device info to show homeplate in device registry#11

Merged
lanrat merged 2 commits intolanrat:mainfrom
elratt0r:add_device_info
Dec 10, 2022
Merged

add device info to show homeplate in device registry#11
lanrat merged 2 commits intolanrat:mainfrom
elratt0r:add_device_info

Conversation

@elratt0r
Copy link
Copy Markdown
Contributor

Quick hack for the homeplate to show up in HA Device Registry via MQTT Discovery

Copy link
Copy Markdown
Owner

@lanrat lanrat left a comment

Choose a reason for hiding this comment

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

Hello @elratt0r, thanks for the PR!

I left a few minor comments, to allow your changes to work on other Inkplate hardware and support multiple homeplates on the same network.

Let me know what you think.

Thanks.

Comment thread src/mqtt.cpp Outdated
StaticJsonDocument<devcapacity> deviceinfo;
deviceinfo.clear();
deviceinfo["manufacturer"] = "e-radionica";
deviceinfo["model"] = "Inkplate 10";
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can this be set using macros so other Inkplate devices like the Inkplate6 can work as well?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

edit: it does not need to be macros. Ideally I'd like to avoid hardcoding a specific model here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll have a look at it, how to get the info for which model the code is build. I think compile time via macro or config.h makes sense because usually you're building it for a specific device. Maybe added to platformio.ini under the corresponding env section?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

As long as its not hard-coded I'm happy with it. Using config.h or platform.ini seem like good options.

Copy link
Copy Markdown

@phidauex phidauex Nov 29, 2022

Choose a reason for hiding this comment

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

You could use the build flags chosen by the build environment (I should have a pull request soon to add more build envs) and then an #ifdef statement - something like this:

#if defined(ARDUINO_INKPLATE6PLUS)
  deviceinfo["model"] = "Inkplate 6PLUS";
#elseif defined(ARDUINO_INKPLATE10)
  deviceinfo["model"] = "Inkplate 10";
#else
  deviceinfo["model"] = "Inkplate (other)";
#endif

The disadvantage here is that a model needs to be added for each plausible build environment. I don't know if there is already a variable somewhere that includes a friendly model name of the board.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Unfortunately I don't think such a model exists yet, so I'm happy to have it added to the platformio.ini file in this repo.

There is an official inkplate platformio repo but it does not seem to be regularly updated or maintained as compared to the Arduino library I use here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I suppose the build env could just define a friendly model name, and this section pulls that definition. If the definition is empty, then a sensible default is used.

Comment thread src/mqtt.cpp Outdated
deviceinfo["manufacturer"] = "e-radionica";
deviceinfo["model"] = "Inkplate 10";
deviceinfo["name"] = "HomePlate";
deviceinfo["identifiers"][0] = "homeplate";
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This should be unique to the specific device in case there are multiple homeplate's on the same network. I think it should be the device's mac address so that device tracker integrations can identify it as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, I think this should be extended to the mqtt topics and unique_ids as well. That would make discovery less error prone. For the activity-topic the hostname could be used (ex. "homeplate//activity"). Any thoughts?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think using a unique name for the activies would be a very welcome improvment. I noted that it should be done a while back but never got around it it. You are welcome to tackle that if you would like. :)

Comment thread src/mqtt.cpp Outdated
deviceinfo.clear();
deviceinfo["manufacturer"] = "e-radionica";
deviceinfo["model"] = "Inkplate 10";
deviceinfo["name"] = "HomePlate";
Copy link
Copy Markdown
Owner

@lanrat lanrat Nov 29, 2022

Choose a reason for hiding this comment

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

Should this be the hostname? (which defaults to "homeplate")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Easiest would be hostname. Extra friendly name seems to be overkill and name can be easily changed in HA.

@elratt0r elratt0r force-pushed the add_device_info branch 2 times, most recently from e0eda55 to 0fc4579 Compare December 2, 2022 10:16
@elratt0r elratt0r requested a review from lanrat December 2, 2022 11:33
@elratt0r
Copy link
Copy Markdown
Contributor Author

elratt0r commented Dec 2, 2022

Modified the patch to make device model, name and identifier configurable by config.h, put sensible defaults in place.
Follow-up patch for use of node_id as unique_id prefix and in mqtt topics is ready (see: use_node_id branch in elratt0r/homeplate)

@lanrat
Copy link
Copy Markdown
Owner

lanrat commented Dec 10, 2022

This looks great, thanks for the PR!

@lanrat lanrat merged commit 6d2d5df into lanrat:main Dec 10, 2022
@lanrat
Copy link
Copy Markdown
Owner

lanrat commented Dec 10, 2022

I wonder why the entities are not showing up in the device info? Its not a problem. Just idle curiosity.

Screen Shot 2022-12-10 at 12 45 46 PM

@elratt0r
Copy link
Copy Markdown
Contributor Author

I wonder why the entities are not showing up in the device info? Its not a problem. Just idle curiosity.

Screen Shot 2022-12-10 at 12 45 46 PM

Weird, can you check the homeassistant/sensor/homeplate topic?

I had to remove the homeplate retained part when I played around with the naming and too many config parts piled up. But that should not happen here as we just added a device info part in the json.

@lanrat
Copy link
Copy Markdown
Owner

lanrat commented Dec 10, 2022

Everything looks correct in MQTT. I wonder if HASS only checks for the device field the first time it is seen?

Screen Shot 2022-12-10 at 1 12 30 PM

Do you see the sensors under Entities in the MQTT devices section of HASS?

@elratt0r
Copy link
Copy Markdown
Contributor Author

scr_hass_sensors
scr_hass_mqtt

I see both. Just for testing I deleted the whole /homeassistant/sensors/homeplate topic. The device disappeared as expected and showed up again after the new config push after flashing.

@lanrat
Copy link
Copy Markdown
Owner

lanrat commented Dec 10, 2022

That did the trick!

I had to clear the homeassistant values in MQTT and then reboot the Homeplate.

Screen Shot 2022-12-10 at 2 12 22 PM

@elratt0r elratt0r deleted the add_device_info branch December 10, 2022 22:45
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.

3 participants