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

Enable multiple Digital & Events #1832

Merged
merged 4 commits into from
Aug 5, 2019
Merged

Enable multiple Digital & Events #1832

merged 4 commits into from
Aug 5, 2019

Conversation

pilotak
Copy link
Contributor

@pilotak pilotak commented Jul 21, 2019

I know that there is going to be support for specifying all the things dynamically through browser but before that here is enhancement to support it now. This would also help for anyone finding #1815 as there is no code provided, only guidance

@pilotak
Copy link
Contributor Author

pilotak commented Jul 21, 2019

The only thing i don't know is how to set pwrRatioE and pwrResetE in web-config for each sensor separately

@mcspr
Copy link
Collaborator

mcspr commented Jul 22, 2019

pwrRatioE<index>? where index is just a _sensors position
Reset & calibration are a strange ones. Wouldn't it be easier to use direct action calls instead of going through settings and then purging keyvalues?

You might want to also add mapping from the old ..._PIN to the ...1_PIN

@mcspr mcspr mentioned this pull request Jul 22, 2019
@pilotak
Copy link
Contributor Author

pilotak commented Jul 22, 2019

pwrRatioE<index> is not a problem to add but how to hide the unused in browser - there is only section hiding (in html there will be all 8 listed but lets say you have only two setup which would need display: inherit).
How do i do "direct action calls"?

@mcspr
Copy link
Collaborator

mcspr commented Jul 22, 2019

Append html settings section programmatically?

By direct actions I meant ws actions:

void _relayWebSocketOnAction(uint32_t client_id, const char * action, JsonObject& data) {

function sendAction(action, data) {

(thus directly requesting required actions instead of using one-shot settings)

@pilotak
Copy link
Contributor Author

pilotak commented Jul 23, 2019

ok, what if i remove pulse-meter from this PR and create a separate once it's done?

@mcspr
Copy link
Collaborator

mcspr commented Jul 23, 2019

sure, that will work

@mcspr
Copy link
Collaborator

mcspr commented Jul 23, 2019

You might want to also add mapping from the old ..._PIN to the ...1_PIN

Btw, by that I meant to add something here https://github.com/xoseperez/espurna/blob/dev/code/espurna/config/deprecated.h
to remap old defines into the new one + show a warning. Release notice will be there too, but at least VSCode / Atom with PlatformIO will track those messages and show them after build completes if someone uses those in custom header

@pilotak pilotak changed the title Enable multiple Digital, Events & PulseMeter Enable multiple Digital & Events Jul 25, 2019
@pilotak
Copy link
Contributor Author

pilotak commented Aug 5, 2019

@mcspr any other request? i think it can be changed otherwise

@mcspr
Copy link
Collaborator

mcspr commented Aug 5, 2019

Don't think so. Any other changes can be made in bulk with other sensors (and maybe even move sensor init to sensor classes themselves)
Sorry for the delay, was I bit busy past 2 weeks.

Also, cc @lbdroid mentioned this #1529 (comment)

@pilotak
Copy link
Contributor Author

pilotak commented Aug 5, 2019

That's ok, can this be merged?

@mcspr mcspr merged commit 228fba5 into xoseperez:dev Aug 5, 2019
@mcspr
Copy link
Collaborator

mcspr commented Aug 5, 2019

Done.
I have noticed some pulsemeter changes left over, but they just chain back into themselves and don't change the existing behaviour

@pilotak
Copy link
Contributor Author

pilotak commented Aug 5, 2019

I know let's call them preparation for future

@pilotak pilotak deleted the multiples branch August 5, 2019 18:11
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.

None yet

2 participants