Add support for motion v4.4

[MichaIng]

Additionally:

• Do partial imports for some modules
• Migrate to f-strings where applicable

ToDo:

• Inform Home Assistant addon developers once this has been merged: Update to Motion 4.4.0? hassio-addons/addon-motioneye#350

[MichaIng]

I need some help guys, the mapping currently contains some dummy conversion functions which need to be finished:

• motion v4.3 to/from v4.4 options mapping is a little tricky.
• All netcam_ options have been merged into a single netcam_params options, with comma-separated key-value pairs as value.
• So for _MOTION_43_TO_44_OPTIONS_MAPPING we need to loop through netcam_ options and concatenate the netcam_params value with respective key-value pairs. The way the mapping works currently, I'm not sure how to do this, or whether we need to do/change it in the parent function.
• For _MOTION_44_TO_43_OPTIONS_MAPPING we need to loop through the netcam_params key-value pairs and assign the respective value to the respective netcam_ option. With splitting this should be possible, but I'd need to check some Python docs and examples 😅.

I hope someone finds time to update/finish this PR accordingly.

Another question:

• Currently the motion version is obtained via external motion -h call for every version comparison.
• This means that now it's called two times whenever camera options are read or set.
• Shall we instead obtain the motion version a single time at motionEye server start, store it in a global variable and use this for version comparisons? Would reduce overhead and would allow us as well to print a warning if the motion version is below 4.1 or above 4.4, i.e. unsupported, which means that some camera options may not work as expected.

[tunkio]

I have one idea for this, but I need to take few days break first. If there is nothing happened until I'm back I can try to do some tests how that idea would work. (Config resolver class type thing)

[MichaIng]

That would be great. Take your time, of course. I'm also busy in new job right now, so don't have the time to look deeper into Python depths for this. Probably on weekend I find some time.

[MichaIng]

Btw, I'm much appreciating your motionEye contributions. Great to have someone which loops through open issues and has good Python knowledge 🙂👍.

[zagrim]

Shall we instead obtain the motion version a single time at motionEye server start, store it in a global variable and use this for version comparisons? Would reduce overhead and would allow us as well to print a warning if the motion version is below 4.1 or above 4.4, i.e. unsupported, which means that some camera options may not work as expected.

This sounds like a reasonable thing to do. 👍 It would make the issue of "too new Motion version" more clear and reduce confusion in that particular case. I'm not sure if I've seen an issue ever reported for too old Motion, but sure that also possible to happen.

Indeed the netcam_* stuff seems a bit tricky, and my thin Python experience is not of any help in this.

[tunkio]

Really fast implementation of the mapper what I told earlier. It's done with older python which didn't support f-strings or there are no typings but, if you think that would be good way to go I'll do conversions.

https://github.com/tunkio/motioneye/tree/motion_config_mapper/motioneye/utils

You can run testdata with motion_config_tester.py (located motioneye/motioneue/) That just demonstrates conversions between different versions.

Under the motion folder there are couple example camera configs 4.1, 4.2 and 4.3. Mappings are done from motioneye -> motion version and motion version -> motioneye.

Yaml is used in mapping files so if there is changes later(motion 4.5) it should work so that you just add new mapping file with changed values.

We could add variable checking there too, now I just listed variable types, defaults and possible values in motioneye.yaml. Didn't implement any verifications yet.

These mappings aren't complete, only for testing.

[tunkio]

Currently the motion version is obtained via external motion -h call for every version comparison.
This means that now it's called two times whenever camera options are read or set.
Shall we instead obtain the motion version a single time at motionEye server start, store it in a global variable and use this for version comparisons? Would reduce overhead and would allow us as well to print a warning if the motion version is below 4.1 or above 4.4, i.e. unsupported, which means that some camera options may not work as expected.

There is cache already.(if I'm looking same function 😮)

motioneye/motioneye/motionctl.py

Lines 38 to 41 in 2b146dd

	def find_motion():
	global _motion_binary_cache
	if _motion_binary_cache:
	return _motion_binary_cache

[tunkio]

Replaced old mapping implementation with this new one(repo given above) and seems to work somehow. Need to check what breaks and so on. 4.4.0 motion seems to get correct camera conf.

Automatic conversion from older/newer version requires that we add version number attribute into motion and camera configs. Configs might be from older/newer version, so we should use proper mapper to parse those.

[MichaIng]

There is cache already.(if I'm looking same function 😮)

Ah, somehow I've overseen this 😅, then all is fine.

if you think that would be good way to go I'll do conversions.

Looks actually like a great long-term solution to cover all such cases. I haven't tested or fully understand the YAML handling yet, but looks like you covered the multi-argument values nicely.

I think there are more motion settings covered than actually used in motionEye, isn't it? Shall we reduce this to have mappings only for actually used settings?

Probably we can add the test to be run with pytest in CI pipeline: https://github.com/motioneye-project/motioneye/tree/dev/tests

Didn't implement any verifications yet.

Since we don't have any verification now either, I think it's fine, especially when a CI test compares the generated config with the expected sample.

[tunkio]

@MichaIng ok! If it looks ok, I will continue this. I forget the verfication right now and remove those extra parameters. I thought that if we have all parameters there, it would be easy just use them if needed later. But you are right, it might just add more confusion. It's not hard to add the new ones later.

One thing what is not implemented into this test repo, is cache :) I do that too.

I think there might be easy way to do automatic detection of the current camera and motion config version. If we do some version checking for the existing parameters, we can do automatic conversion from old(example 4.2) -> new(4.4) or new(4.4) -> old(4.2) when we start motioneye. This means that you can update motion itself and just restart motioneye and automatically get updated configuration.

I tested this early implementation and in my tests 4.4 worked well, parameters got proper values(list ones), same with 4.2. There might be problems what I didn't see but most things worked as expected.

[tunkio]

@MichaIng if you have time 😄 and motivation you could test version below from my repo.

https://github.com/tunkio/motioneye/tree/motion_config_mapper

I added python script for updating existing motion and camera configs. It might take too much time for now to make that automatic. Script is located at motioneye/scripts/update_motion_config_version.py <from_version> <to_version>

It will update motion.conf and camera-x.conf files in /etc/motioneye/ folder. Automatic backup will be taken into /etc/motioneye//. Downgrade and upgrade possibility.

I didn't remove unused configurations from yaml files yet. We should keep all config differences so we can convert configs between different motion versions.(like 4.1->4.4 or 4.4->4.1).

Update script uses functions from config.py.

You can run server as well with different motion versions.

[MichaIng]

I implemented a solution for the netcam parameter mapping, hope it works, will test thoroughly before opening for review. Netcam URL, user and password parameters actually do not need to be translated, they are still separate settings in motion 4.4.

Passing data to the individual migration functions was btw not required, until now 🙈: The v4.3 to v4.4 migration for netcam params uses it now to append further params to the existing value.

A migration script would still be nice. Also there is already one: https://github.com/motioneye-project/motioneye/blob/dev/motioneye/scripts/migrateconf.sh
However, an automated migration which does not require any manual steps, is what I wanted to achieve here.

What is still required now when upgrading motion is to save camera settings once. They are always internally migrated to motion 4.3 settings when loaded from config file (from any motion version), and saved to config file s v4.4 settings, if this is the motion version.

[MichaIng]

Works great now on all my tests with all possible parameter types. I also updated the motioneye_init script to pull motion v4.4 if available. Since neither Debian, not Ubuntu ship motion v4.4 yet, it is now tried to be downloaded whenever the distro code name can be detected (else it is doomed to fail), but gracefully handles the case where no matching package is available or the download fails. E.g. there is no Bullseye package for ARMv6hf (RPi 1 and Zero (1)) available yet. In this case the package from the distro's repository will be installed as fallback.

@xl442
Ah dammit, I've overseen your patch 🙈. However, at least it works now with minimal change to the previous mapping method. We must keep this PR in mind in case better methods are wanted or required with future motion versions, and we have two other nice approaches here.

[MichaIng]

@tunkio @zagrim @xl442
Do you find time for a test/review? I would like to push this, add some more informational output to motioneye_init and then push a pre-release (on GitHub + PyPI) until this weekend.

[zagrim]

I fear that I won't have time/energy to really test this, but surely I can try to review it (just not right now, I've exhausted my brain for today).

[zagrim]

Not being that well versed in Python might have neglected some details, but overall I think it is ok.

While validating the 4.3 to 4.4 mapping I noticed that there's _USED_MOTION_OPTIONS which is used for separating out Extra Motion Options. That list has not been updated, so I wonder how well it works anymore with Motion 4.4? In theory some 4.4 options that ME does use might leak to that UI field... 🤔

[MichaIng]

You are right, the extra motion options users may have set (manually) are not translated and were not before. We'd need to have a look how those are applied and do param split and mapping there, but it's probably too much for now?

[zagrim]

You are right, the extra motion options users may have set (manually) are not translated and were not before.

That's one aspect of it, and I agree that it is perhaps too much to try to do complete config migration when we don't really have anything to check the result against.

But my point was that _USED_MOTION_OPTIONS list is used by motion_camera_dict_to_ui to select what goes to Extra Motion Options, and by motion_camera_ui_to_dict to (over)write config with what is in Extra Motion Options. Or did I miss something that prevents, for example, from "video_params" (which is the result of migrating "vid_control_params") from ending up in Extra Motion Options when motion_camera_dict_to_ui is next run, since _USED_MOTION_OPTIONS contains the old "vid_control_params" but not the new "video_params"? This is of course something that could be checked by testing the migration, which I can't find the time for now.

[MichaIng]

Ah, I'll have a closer look at that. Several tests on all directions however went well and I always checked the camera config file before and afterwards.

EDIT: Ah I think this is the reason why those parameters ended up at the bottom of the file with a blank line separated, however correctly mapped. I need to have a look that they do not end up in the UI as extra options, but the mapping is actually done first and it would have been the same issue before.

[MichaIng]

I had a closer look into where _USED_MOTION_OPTIONS is used. And as far as I can see it is indeed only used after the config dict has been converted to motion v4.3 already, respectively before it gets converted to local motion version, hence it is applied correctly. Also I do not see any issues when testing it back and forth with different motion versions.

By times some more code comments and probably resorting/factoring would be beneficial, to make it easier for contributors to understand in which function what is done and why it is done in that order/stage of reading/writing configs from/to config files. Also there seem to be quite much redundancy and potential for enhancements, for the future 🙂.