Community
OpenEnergyMonitor

Community

Development: Devices, Inputs and Feeds in emoncms

device
development
emoncms
Tags: #<Tag:0x00007f16b3103f80> #<Tag:0x00007f16b3103e40> #<Tag:0x00007f16b3103d00>

(Paul) #82

I specifically need to edit the devicekeys, the generate new is a great addition but it doesn’t help when you have devices in the field and you are marrying them up to an account.

Think if you were to sell emonTx or emonESP’s with preset keys, users need to be able to add that to the device config page. I know you are thinking the devicekey could be taken from the emoncms page and added to the device, but that assumes you have access to it.

So are you saying the numerical ids are here to stay in emoncms? That’s confusing

if we are not or can not go over to text process names that’s another thing, but it does seem like a good route to go, the process lists are extremely difficult to read, here’s one I picked at random that’s not out of the ordinary.

I have to keep a spreadsheet “cheatsheet” open with all the process ids in so I can read processlists.

What happens when we use processes that do no have an id eg eventp and schedule? is the lookup robust enough to know not to use an id?


(Trystan Lea) #83

Hmmm, ok the possibility of using text based process keys seems to have been an addition by @nchaveiro that I missed. Back to the drawing board then! From the device modules perspective perhaps we just print the processList as is for now in the log.


(Paul) #84

Hopefully it’s not quite that bad, is it?

I think the log output will be more useful if it’s true, proper formatting helps but the main thing is to be able to trust what you see. If the process ref are numeric then so be it, I would like to work towards text based and then the logs will follow suit naturally.

Is there a performance penalty with the text based refs? Obviously we don’t want to pursue it if it’s not efficient, but with regard to interacting with emoncms, it would be so much better to have text based refs.

So it the original device module could process text based refs, and emoncms can too, can we continue this in the device-integration branch?

Do we need to convert text to int ids?

Will this help stop the processlists being overwritten with every init?


(Trystan Lea) #85

Hello Paul

I’ve reverted the process log to show what is actually saved, with a new line between between each item and I’ve added in the ability to set the device key. I’ve added validation to ensure devicekeys are of a similar format to the devicekeys produced by the random generator, 32 characters and alphanumeric. I realise this could break backwards compatibility but would like to ensure the devicekeys cant be as simple as 123 or ‘mydevicekey1’, Im happy to reduce the minimum length a bit if its a big problem, or we could break it out into a device module setting perhaps…

As far as I can tell from the code (@nchaveiro and @adminde wrote this part) process lists are currently always overwritten, the content of the existing processlist is not checked, it looks like that’s the case in the master branch as well so I dont think we have changed the behaviour in the device-integration branch…

Im keen to get this to an initial release point. I had a quick go at implementing the dry run feature but its more complicated than I though as the building of input and feed processes rely and inputs and feeds having been created so that their ids can be written to the process lists. I therefore think this feature should be parked for a subsequent release.

We’ve got plenty of new developments in this release as it is, so I think we should avoid trying to add further new features in at this point and just ensure that we have not introduced any new fundamental bugs.


(Paul) #86

Correct, the original offering wasn’t multi-init, the checks to the input and the feeds will probably have come with the new features, so I would class the absence of the processlist checks as an unfinished multi-init feature as is only performs half the checks, only input and feeds, not input processlists or virtual feed processlists.

Me too, but we need to know it is usable. We only found all these issues by accident because I had felt compelled to update my live instance because of the emoncms security patch and then quite by chance needed to set up a new account with new devices. Had things not gone south at that point we might not have known about the issues before merging. I’m afraid I will not be pulling these changes into my live server yet so testing is going to be slower and less “real world”. IMO it is best kept in a feature branch until there is confidence that if does what is expected.

It is also so much easier to switch branches to compare performance/features and if needed “bail-out” if there’s a problem, not so easy to reset the branch to an earlier known good point commit.

We now have the log messages so that will help with testing. I will try and run some tests over the weekend if I get some time.

Agreed, but it would be nice for those to be complete before merging, checking the processlists isn’t another feature, it’s part of the “only edits if there’s changes” feature.

Agreed, the dry-run can be implemented later, as can a template viewer etc, (although I consider this essential - but I keep a ssh console open so I can check the template before init and the “dry-run” could allow more intensive testing without creating thousands of new feeds and inputs.

Absolutely, but ideally before it is merged to master or “released”, at this stage you can still make significant changes and/or change direction, once in the master branch it would be unfair to drop features or drastically change things again.

You’ve made lots of bugfixes and changes since we tested, please lets give them a thorough testing before totally committing to a full release.

[edit]
I guess the processlist checking isn’t essential now we can see the changes about to be made in the log, we can abort rather than over write.

I’m still on the fence about over writing edited processlists, I think that does need more thought, but we still need to thoroughly test what we have now.


(Trystan Lea) #87

Great thanks!

The log is not a log of changes about to be made, its a log of the changes that have been made.

Perhaps this needs a warning, saying re-initialisation will overwrite any changes to device processlists. Are you sure you want to continue? Otherwise I think its a useful feature to overwrite processlists as it can be used to correct misconfiguration which would be the main intention of re-initialisation.


(Paul) #88

Oh **** of course they are!

I think that is a very wise thing to do when hours of writing processlists are at stake.

I agree, hence my “on the fence” position and why I previously suggested when a dry-run is performed, the output includes a checkbox for each line indicating if it will be updated of not, the user can then select/deselect items to alter the list. eg you change a template to include an additional input (eg emonTh payload extended) with new feeds and processing etc and a user wants to pull those changes in because they have updated their emonTH FW, if the device module detects some edits made by the user to the original input’s processlists, it will flag them as “differences” and propose overwriting. The user can deselect overwriting the original processlists (since there are no revisions in those in the template) and still pull in the new input/processlist and feeds etc AND retain all their edits to the processlists.

Without this, the device module basically enforces users either have exactly what is laid down by OEM in the templates and nothing else or not use them at all, or even the device module unless they write their own templates. This seems a little in flexible and may restrict uptake to just “out of the box” users.


(Trystan Lea) #89

Thanks Paul,

Great, I will add in a warning.

Aha so your thinking of something much more comprehensive than I had in mind. It might therefore make sense to have the process of initialising a template run from the client side in javascript rather than a server side php function is is currently the case. As the user works through the list the client javascript would then send create feed, set process list etc requests one step at a time. Its a nice idea.


(Paul) #90

I’m not sure that would work. IMO the changes need to be assessed as a complete task, say there was a processlist that depended on a feed and the user deselected the feed, that could cause and issue.

I thought the “dry-run” would run through the template making all it’s checks, print a list (like the current log but) with tick boxes and a “confirm” button. If a user changed any tick boxes the button changes to “re-check” and again prints the list with the revised changes and a “confirm” button. Whether this is done server side or client side I will leave to your better judgement, I guess once the “before” feed and input lists are fetched the checks and negotiation with the user could be done at the client side and then the final “confirm” could send the actual requests for the changes. My only concern here is if there are multiple users (or scripting using api calls etc) that there is a chance of changes to emoncms during the checks and “negotiation” which might lead to failures, so yet another check might be required just before persisting the changes?


(Paul) #91

Further to this I had a thought yesterday that we could implement something like a device module “ignore the rest” processlist marker process, this process would be automatically added at the end of every processlist adde/edited by the device module (even if absent from the template.

beyond this point in the processlist a user could set up any additional processes (using reset to input, +feed etc first), the device module would only compare the template with processes upto and including that marker process.

In fact if the marker process could even include a “reset to input”.


(Paul) #92

Sat down to do some testing this morning with a fully updated device module and 9.8.22 emoncms.

Since I wanted try various things with some test templates I decided to start by separating any test templates from the included templates, both in location and menu grouping, plus test the ability to use sub-folders whilst I was at it.

So I created a “pb66” folder in device/data and moved the existing “test” (copy of emonth) template there. I then edited that template so category=“pb66 templates” and group=“Test and Development”.

I have cleared browser cache, I have navigated away and back to the device page, I have also tried logging out and back in, but nothing I do changes the left hand panel of the device create/editing page. OpenEnergyMonitor:Temperature and Humidity:test is still listed and active.

I have checked redis and found the old key:values is still there and the new (moved and edited “test”) template is not, just in case the sub-folder wasn’t being read, I copied the “data/pb66/test.json” to data/test_copy.json and that has failed to make an appearance too.

At this point I assume it’s just a case of redis still not being updated at the right time so no edits, removals or additional templates are picked up.

My current device/data folder looks like this

data
├── amig.json
├── amrm.json
├── OpenEnergyMonitor
│   ├── emonpi-HEM.json
│   ├── emonpi-SPV1.json
│   ├── emonpi-SPV2.json
│   ├── emonth.json
│   ├── emontx-HEM.json
│   ├── emontx-HP.json
│   ├── emontx-SPV1.json
│   └── emontx-SPV2.json
├── pb66
│   └── test.json
├── smartplug.json
└── test_copy.json

but the menu and device templates listed on the device creation/edit page has not altered from when we were testing the other day.

Previously any template changes could be dragged through by creating a throw away device, so I have tired that here and created a device called “JustToUpdateRedis” based no the (no longer existing) OpenEnergyMonitor:Temerature and Humidity:test template, the device was successfully created but no changes were dragged through, the list remains unchanged even after clearing the browser cache. So it would appear not only is it still faulty, but the previous work around is no longer available either.

Intrigued by this I then deleted the “domp:device:template:smartplug” key:value from redis in the same way I was deleting devices the other day, to see if it popped back up after creating a another dummy device, however now I have no templates at all listed on the left panel so I cannot create any devices thorough the webpage, haven’t tried an api call yet, but I’m out of time for now, will try to get back to this later.


(Trystan Lea) #93

Thanks for testing Paul. Good spot, I’ve modified it now to reload the device templates when you open the device view page (refresh) rather than pulling from the redis cache (@adminde hopefully Im not undoing your intention for caching by reloading at this point?) I’ve also forced a javascript reload by appending version numbers which will help with browser cached javascript.

Changing the folder of the device templates does not affect the grouping in the device view, the grouping is set in the device template itself.

{
    "name": "AMRM",
    "category": "Datalogger",
    "group": "ArchiMetrics",

You could put this device template in a folder called Datalogger or leave it in the root data directory, either way it will still appear under the category Datalogger as set in the device template.

The recursive folder search just loads all the devices as if they where all in the root data folder…


(Paul) #94

Thanks Trystan, I will try and get back to this soon, I currently have my plumbers cap on and all the upstair’s floors up renewing all the pipework, so I need to focus on that just now.

I guessed that was the case from the existing included templates but was unable to confirm anything with the redis issue at large.

I will report back once I’ve had another stab at it.


(Adrian) #95

Hi guys,

sorry for not having reacted this long.

Absoluteley not. This is probably the best way to go here. Currently device/template/list will reload all devices, while device/template/listshort will only return cached meta data. This “manual reload” by calling a specific API request was never viable^^
Main idea for the cached meta data was actually to enable modularization for the templates (as the meta data contain the “module”, where to look for the module_template.php file) and other flags, like available control parameters.
It would probably be a “clean” solution to provide a $device->load_device_templates() function, reloading the templates and returning the meta data, instead of clearing redis cache in the controller. For non-redis instances this wont even work.
I can do this ASAP :slight_smile:

You probably noticed this while developing the first dry_run branch, but the main thing I did change in the initialization of @nchaveiro was to check if feeds or inputs exist and continue with their ids if so. Beforehand, every feed or input was just created and ignored if the function returned an error (which was naturally the case, if already existing). This “re-initialization” could easily be skipped again for what its worth, if it is causing that much hassle^^


(Trystan Lea) #96

Thanks @Adminde that would be great!

I think we leave it as we currently have it, the re-initialisation as it currently stands is useful in my opinion, we just need to add a warning to ask the user to confirm.


(Trystan Lea) #97

I’ve added in the warning for device re-initialisation.

@pb66 have you had a chance to do any further testing?


(Paul) #98

Hi @TrystanLea, No I haven’t had much of a chance, but I will make time to do so in the next few days or so if you like.


(Trystan Lea) #99

Thanks Paul, that would be much appreciated!


(Adrian) #100

Hi again,

I kinda found the motivation recently, to play a little with the dry-run implementation and opened a pull request.
Sooo, @pb66 or @TrystanLea I’d highly appretiate it if you’d maybe find some time at any point to look into/test it as well :slight_smile: and maybe this helps to bring the whole device-integration branch to a more production-like state.


(Paul) #101

Sounds great!

I hope to do some further testing over the next few days, perhaps @TrystanLea could pull these changes in so I can test it all together?