Tzachi shared node phase2

[tzachi-dar]

This PR adds 4 more modules to the shared node:
oref0-calculate-iob
oref0-meal
oref0-get-profile
oref0-get-ns-entries

Technically this changes are not complicated, but looking at the PR it seems to touch a lot of files (many of them only have one line changed).

The main reason for the many changes are this:

1. All files that are touched by this pr (or called from files running by this PR) are moved to strict mode.
   Usually, this only means to add strict to the beginning of the file. Sometimes this meant that variables have to be declared in functions. In a few places variables were shared across files, so I had to make sure they are passed as parameters.

2. There are some functions that have functionality that needs to be moved to the calling function.
   This includes the following:
   console.log(), console.error() and process.exit()
   This 3 functions have been replaced by console_log(), console_error() and process_exit().
   The new functions only receive one extra parameter (final_result), that will hold the data to be returned to the calling function.

There are more printings in the code that I did not move to the new paradigm in order not to make this PR even longer.

Obviously, this PR needs a through code review and more testing.

[scottleibrand]

Just getting back to this. Has anyone tested this on a rig yet? If so, on a Pi, Edison, or both?

[scottleibrand]

So I take it this is still untested on any production rigs?

[tzachi-dar]

Correct, it is tested on a rig, getting production data and controlling a pump, but another pump is actually closing the rig.
The input and output data are being calculated with the new code and the old code and then they are compared.

[tzachi-dar]

How is review going? Will it help if I split this into smaller prs?

[scottleibrand]

I think the next step is to install it on a rig controlling a live pump and make sure it still works correctly. I had done such testing against #1359, but it sounds like you abandoned that, so I'm unclear whether any of this code has been tested or not. I haven't gotten back to it since you closed #1359: have you had any chance to do any live testing since then?

[tzachi-dar]

#1359 was merged into dev as part of #1361.
The question is how to continue form there. People that are installing dev these days are running with this code.

This PR moves 4 operations (for example oref0-meal) to the shared node. Since these operations can be categorized as programs that read input files and write output files I think that the best way to move on is to create a very small pr that only copies this files. Once we have enough test data, we can run the old code and the new code and make sure that they are exactly the same.

Does this sound reasonable?

[scottleibrand]

Ah, ok, thx.

Yes, that is a good test plan, and more thorough than my "just try it on a rig and make sure it still works" method. If you don't have cycles to get the former done, I'll probably be able to do the latter at some point after I finish my taxes. ;-)

[tzachi-dar]

OK, so I'll prepare the scripts needed to collect the data until the end of the week, so we can start collecting.

[jimrandomh]

Not sure what the next step with this branch is. #1411 addresses the root cause of the performance problems with node, which were a big part of the motivation for this branch. But while the interpreter startup performance with a fixed nodejs interpreter is better, it's still not great, and wouldn't be able to handle calling nodejs scripts in a loop. And there are some medium-to-long-term architectural advantages to having a persistent nodejs process, in the ability to preserve state between loops without using hacky JSON files in ~/myopenaps.

[sulkaharo]

@jimrandomh OpenAPS architecture is (AFAIK) intentionally based on the functional programming paradigm and thus the logic is designed to be inherently stateless. When discussing this branch / architecture changes originally, I recall the only state that was discussed was retaining in-memory copies of the deserialised files and only reloading files from disk if needed. Having said that, the logic in OpenAPS hasn't been implemented functionally and does direct changes to the data as well as uses globals, so having more logic live in the process would require careful refactoring of the code. OTOH this would probably speed up running the loop to a fraction of the time currently needed and personally I think not relying on the rig running a particular version of the shell would in my opinion be an architectural win.

[tzachi-dar]

@sulkaharo
This PR does not change the logic of storing/reading files from disk. In all places that files have been read from disk (and cached), the code has changed to always read the file (a one line change).
There have been a few global variables but their usage if fixed now (I hope).
There is code that runs on the unmodified version of the code, and copies the input files and the output files.
The files are then used to test the new code, and I did not see any change in the output either.
What we need right now is a comprehensive code review and more testing.
By the way, a few of this changes are already live in dev branches of the pi, so architecture should be fine.

[sulkaharo]

@tzachi-dar Ah - I was referring to Jim's notion of expanding more (all?) of OpenAPS run in the persistent process. Related: there's a new Pi Zero 2 out now that apparently has 5x better CPU performance. This comes at a cost of higher battery draw under load, so I'm assuming if people change over to the new Pi in their rigs, the point of these optimisations would be to have the CPU spend less cycles to draw less current, not to enable the loop to run in the first place.

[scottleibrand]

Extraneous logging when calculating meal.json:
Pump history updated through 2021-12-17T20:17:05-08:00 with 4 new records; meal.json dir_name = /root/test_data/oref0-meal2021-12-17-2018

[scottleibrand]

After commenting out extra echo lines, this runs successfully on a rig with all and only the expected output.

[scottleibrand]

@tzachi-dar is this ready to merge to dev from your perspective? Is it still useful after the changes in #1411?

[tzachi-dar]

This PR is ready to be merged from my prespective.
The solution introduced in this pr is better than the one in pr#1411.
After 1411, it still takes around 1.5 second to start a node running. (please note that the code is being compiled each time when starting node).
As far as I can remember from my tests the average load on the rig was around one with my pr, and is around 2 now.
That said, I don't have an environment to test this right now.
That said after #1411 the rig works much better. so I guess that motivation to invest in this direction is much lower.
If you have the BW to test this as well, it would be great, but I will understand if you fill that it is not that important right now.

[scottleibrand]

I'm now running this on all of our rigs, and have not seen any issues so far. If they continue to look good and I don't hear any objections, I'll probably merge it to dev tomorrow.

[scottleibrand]

I haven't seen any issues related to this branch on any of our rigs. They are all Edisons, and seem to be marginally faster, although it's hard to tell for sure, as they were pretty quick already.

[scottleibrand]

After merging this, every rig updated to the latest dev will need to have oref0-setup re-run (normally via ~/myopenaps/oref0-runagain.sh) to get the new shared-node setup installed properly so the rig can resume looping.

[tzachi-dar]

Are you sure a setup is needed?
The shared node already started to run in a previous pr, so although this adds much more code, the shared node is already running, so I don't see why running setup again.
We also have the file "oref0\bin\oref0-upgrade.sh" that can be used in order to do additional steps if they are needed.

[scottleibrand]

I don't know the precise conditions under which oref0-setup is required, but I know that I upgraded a few rigs that were on much older versions of dev, and they stopped looping until I re-ran oref0-runagain. I think it's fine to require people to do that manually: we shouldn't expect to be able to just git pull without reinstalling unless the changes are very minor.

[tzachi-dar]

OK, makes sense.