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

Separate Device Group Name Settings #8082

Closed
pcdiem opened this issue Apr 6, 2020 · 5 comments
Closed

Separate Device Group Name Settings #8082

pcdiem opened this issue Apr 6, 2020 · 5 comments
Assignees
Labels
enhancement Type - Enhancement that will be worked on

Comments

@pcdiem
Copy link
Contributor

pcdiem commented Apr 6, 2020

Have you looked for this feature in other issues and in the docs?
Y

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is.
PR #8014 adds the ability to subscribe to multiple MQTT topics. The same MQTT topics are used by modules that control multiple device groups (PWM DImmer). Being able to subscribe to multiple MQTT topics is a great addition for all devices, not just those that don't use device groups. Also, the purpose of GroupTopic2, 3 and 4 is now confusing.

Describe the solution you'd like
A clear and concise description of what you want to happen.
I'd like to propose adding settings text values specifically for Device Groups.
In order to enable PR #8014 to work with devices that support multiple device groups, I believe we should split the device group name(s) off into separate settings rather than continuing to use the MQTT topic(s).

This would also support and upcoming PR I plan to submit that implements the DevGroupSend command which allows sending updates to a device group via a command. This command will likely be used in rules to send updates to device groups other than the one(s) the device is a member of. This will require defining the device group names of any device groups the user plans to send updates to because we can only reliably send to known device groups (since we need to not just send a packet but make sure the packet is received and acknowledged by the known members of the group).

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
I could continue to use GroupTopics2, 3 and 4 but that prevents devices that control multiple device groups from taking advantage of PR #8014 and keeps up the somewhat confusing dual-use of GroupTopics.

Additional context
Add any other context or screenshots about the feature request here.
I used the MQTT topic when I wrote device groups support because it was logical that the device group name would match the MQTT topic. However, when a device supports controlling devices in multiple device groups, there is no logical tie between multiple MQTT topics and multiple device group names.

(Please, remember to close the issue when the problem has been addressed)

@arendst
Copy link
Owner

arendst commented Apr 6, 2020

I agree to add a command DeviceGroup or GroupName with up to FOUR groups similar like the current GroupTopic.

@arendst
Copy link
Owner

arendst commented Apr 6, 2020

If you manage to move all code from support_device_groups.ino to a new xdrv_38_device_groups.ino file it makes integration easier as the new command code could be included (and disabled if needed) in this driver file just like any other driver.

It might be challenging with regards to compile time as files tend to be compiled in alphabetical order and you may encounter issues here (which might be solved with prototypes).

@arendst arendst added the enhancement Type - Enhancement that will be worked on label Apr 6, 2020
@pcdiem
Copy link
Contributor Author

pcdiem commented Apr 6, 2020

How about DevGroupName so it's consistent with the existing DevGroupShare and the, hopefully, upcoming DevGroupSend?

I was thinking the support_ modules were code that is used by multiple driver/sensor module. The existing DevGroupShare command is not compiled in if USE_DEVICE_GROUPS is not defined. If you still feel it's worthwhile, let me know and I'll try to move it to xdrv_38_device_groups.

@arendst
Copy link
Owner

arendst commented Apr 6, 2020

Ok. Keep it as it is for now. DevGroupName is fine.

@pcdiem
Copy link
Contributor Author

pcdiem commented Apr 8, 2020

Done. Add DevGroupName command #8087

@pcdiem pcdiem closed this as completed Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Type - Enhancement that will be worked on
Projects
None yet
Development

No branches or pull requests

2 participants