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

Update domoticz to v2020.2 #4674

Merged
merged 9 commits into from
Jun 19, 2021
Merged

Conversation

hgy59
Copy link
Contributor

@hgy59 hgy59 commented Jun 11, 2021

Motivation: Domoticz does not run and does not use generic serivce support yet (and jadahl.com will not create further updates).
Linked issues: #4069, #4124, fixes #4673, (older issues; #1478, #1769)

Checklist

  • Build rule all-supported completed successfully
  • Package upgrade completed successfully
  • New installation of package completed successfully

TODO:

  • cross/jsoncpp fails for arch-qoriq (fails only in context of spk/domoticz, build succeeds in cross/jsoncpp)
    Finally solved by fixing the CMAKE CFLAGS for PPC Archs (ppc arch has multiple cflags, issue is is for newer cmake of native/cmake only as building in cross/jsoncpp native/cmake is not used)
  • domoticz shows undefined version information (Domoticz V2020.2 (c)2012-1970 GizMoCuz and Build Hash: 0, Date: 1970-01-01 01:00:00)
    This is by design, as domoticz does not support build from source package, but only within a git repository (see [Question] no version information when compiling downloaded source domoticz/domoticz#4859)
    Solution: create custom domoticz version information. As we cannot support automatic updates anyway this solution helps to display reasonable version info.
  • add Open-ZWave support (this was hard to get, as both - openzwave and domoticz - have some nonstandard/unusual build requirements).

Remarks

  • update domoticz and add new dependencies
  • use generic service setup (includes DSM7 compatibility)
  • add native/cmake, as domoticz requires cmake >= 3.16.0 and debian development environment has 3.13 (closes spksrc.native-cmake.mk #4321).
  • add Makefile variable to use native cmake
  • update boost to v1.66.0 (v1.69.0 was also successfully built, but reverted to v1.66.0, the minimal version required by domoticz)
  • remove cross/busybox and remove dsm5->dsm6 (user) migration
  • we will not bundle usb drivers with domoticz package (as jadahl.com did), to separate specific drivers from generic packages.
  • we will not support automatic updates of domoticz, as there is no build environment available on the diskstations.
  • newer version of domoticz (2021.1) fail to build
  • deluge packages fail to build with the boost library update

@hgy59
Copy link
Contributor Author

hgy59 commented Jun 13, 2021

domoticz is now ready for testing.
Packages for all archs are downloadable as artifacts of the latests github action build (Checks). All supported archs successfully built domoticz. But deluge fails now due to the boost library update.

Still todo:

  • validate mosquitto package update
  • optional: fix deluge build with boost update (deluge currently builds only for DSM7 for aarch64 and armv7)

@BenjV
Copy link

BenjV commented Jun 14, 2021

@hgy59 Can you tell me were/how I can get a domoticz.spk to test with?

@hgy59
Copy link
Contributor Author

hgy59 commented Jun 14, 2021

@hgy59 Can you tell me were/how I can get a domoticz.spk to test with?

Under "Actions" select a terminated "Build" (not Lint) Action. Under "Artifacts" you can download a zip archive containing all successfully built spk files.
https://github.com/SynoCommunity/spksrc/actions/runs/933333272

@BenjV
Copy link

BenjV commented Jun 14, 2021

I have installed and tested domoticz and it works fine beside some minor issues.

Because the package is running as a user and not (like jahdal's package) as root, the package has no privileges to access usb devices and that is of course needed.

I would suggest during installation of the package to create a group called "domotica" and make the user "sc-domoticz" a member of that group.
Further add a udev file like "70-domotica.rules" to the folder "/usr/lib/udev/rules.d"
The following lines should be in this file:

KERNEL=="ttyACM*", ACTION=="add", GROUP="domotica", MODE="0660"
KERNEL=="ttyUSB*", ACTION=="add", GROUP="domotica", MODE="0660"

And of course these rules should be reloaded with this command:
udevadm control --reload-rules

Some minor remarks:

Please add the loglevel to the service command, otherwise all measurement data domoticz is getting will be logged.
So add to SERVICE_COMMAND in de service-setup
-loglevel 2
This will log only errors not all measurements

In the service-setup there are some definitions used that are available as package variables like:

USER="domoticz"
SERVICE_PORT="8084"

They are already defines in the INFO file, so it would be better to use the package variables for those like this:

USER=${SYNOPKG_PKGNAME}
SERVICE_PORT=${SYNOPKG_PKGPORT}

One last remark.
Maybe you could add the location of the database as a choice during installation via a wizard rule.
Lots of people would like the database on for example a usb disk, because the constant logging domoticz does so preventing the main disk to go to sleep.

hgy59 added 8 commits June 14, 2021 23:22
- update domoticz and add new dependencies
- use generic service setup (includes DSM7 compatibility)
- add native/cmake to fulfill minimal required version for domoticz build
- add Makefile variable to use native cmake
- update boost to v1.66.0
- remove cross/busybox and remove dsm5->dsm6 user migration
- patch source code to show reasonable version information
- add openzwave (needs some workaround)
- add boost:serialization for cross/cereal
- update mosquitto
- remove busybox stuff (and dsm5->dsm6 user migration)
- use BUILD_DEPENDS instead of DEPENDS to build native/cmake first
- fix mosquitto for DSM7
- update cross/c-ares
@hgy59 hgy59 force-pushed the update_domoticz branch from d93c2e0 to 2c66760 Compare June 14, 2021 21:23
@hgy59
Copy link
Contributor Author

hgy59 commented Jun 14, 2021

@BenjV Thanks for your detailed testing.

The intention is to install the package for user:group = sc-domoticz:sc-domoticz.

I was focused on DSM7 where the privileges file is created by the framework on demand.
This is now fixed for DSM6.

And I still have the focus on "advance packages for DSM7".


Further add a udev file like "70-domotica.rules" to the folder "/usr/lib/udev/rules.d"

This will not be supported with DSM7 and must be configured manually by a user of the admin group.
As DSM7 (still with RC1) does not allow to use any different user/group in the "ctrl-script" section, this will be the new standard, and I want to keep this for DSM6 too.


They are already defines in the INFO file, so it would be better to use the package variables for those like this:

USER=${SYNOPKG_PKGNAME}
SERVICE_PORT=${SYNOPKG_PKGPORT}

USER is not used and SERVICE_PORT variable is already used for the SERVICE_COMMAND


One last remark.
Maybe you could add the location of the database as a choice during installation via a wizard rule.
Lots of people would like the database on for example a usb disk, because the constant logging domoticz does so preventing the main disk to go to sleep.

Good idea. As we currently have no solution for shared folders in DSM7, I propose to postpone this for a later update.


Please add the loglevel to the service command, otherwise all measurement data domoticz is getting will be logged.
So add to SERVICE_COMMAND in de service-setup
-loglevel 2
This will log only errors not all measurements

Okey, will add this.

@BenjV
Copy link

BenjV commented Jun 14, 2021

I have tested domoticz only on DSM 7-41882

If you change the group name in the udev rules to sc-domoticz everything will work fine.
So those lines will become:

KERNEL=="ttyACM*", ACTION=="add", GROUP="sc-domoticz", MODE="0660"
KERNEL=="ttyUSB*", ACTION=="add", GROUP="sc-domoticz", MODE="0660"

If you do not add this to the package lots of people will not be able to create those rules and it is very simple for the package to do this during installation.
Just add the rules file to the package and move it during installation to /usr/lib/udev/rules.d folder and add this command to the script
scriptudevadm control --reload-rules

Below a part of the service-setup:

SERVICE_PORT="8084"
# start-stop-status script redirect stdout/stderr to LOG_FILE
LOG_FILE="${SYNOPKG_PKGVAR}/${SYNOPKG_PKGNAME}.log"
# Service command has to deliver its pid into PID_FILE
PID_FILE="${SYNOPKG_PKGVAR}/${SYNOPKG_PKGNAME}.pid"
# domoticz service definition

DOMOTICZ="${SYNOPKG_PKGDEST}/bin/domoticz"
DB_FILE="${SYNOPKG_PKGVAR}/domoticz.db"
WWW_PORT="-www ${SERVICE_PORT}"
WWW_ROOT="-wwwroot ${SYNOPKG_PKGDEST}/www"
WWW_DISABLE_HTTPS="-sslwww 0"
WWW_OPTIONS="${WWW_PORT} ${WWW_ROOT} ${WWW_DISABLE_HTTPS}"

SERVICE_COMMAND="${DOMOTICZ} -daemon ${WWW_OPTIONS} -dbase ${DB_FILE} -userdata ${SYNOPKG_PKGVAR} -pidfile ${PID_FILE} -noupdates -log ${LOG_FILE}"

As you can see the SERVICE_PORT is hard coded to 8084 and used in WWW_PORT which is used WWW_OPTIONS which is used in the SERVICE_COMMAND
So this line would be better.
WWW_PORT="-www ${SYNOPKG_PKGPORT}"

By the way the privileges files are not new, they also work exactly the same way on DSM 6 as on DSM 7, the only difference is that in DSM 7 they are mandatory.

Good idea. As we currently have no solution for shared folders in DSM7.
Can you tell me what you mean by this?
Maybe I can help

@hgy59
Copy link
Contributor Author

hgy59 commented Jun 14, 2021

Can you tell me what you mean by this?
Maybe I can help

A short description is under 6) in #4524

The wizards for shared folders must be redesigned as the "data-share" resources allow only the name of a share (without /volume? and no subfolders), and DSM7 allows access to such folders only.

The target is to redesign the wizards of all packages with shared folders to use resouce definition for DSM6 and DSM7.

@hgy59
Copy link
Contributor Author

hgy59 commented Jun 14, 2021

As you can see the SERVICE_PORT is hard coded to 8084 and used in WWW_PORT which is used WWW_OPTIONS which is used in the SERVICE_COMMAND

The service port is NOT hardcoded, it is only defined in spk/domoticz/Makefile.

The top 5 lines in the service-setup file are created by the framework based on Makefile variables and generic installer code.

The service-setup.sh source file starts with # domoticz service definition and uses the variable SERVICE_PORT.

@hgy59
Copy link
Contributor Author

hgy59 commented Jun 14, 2021

Just add the rules file to the package and move it during installation to /usr/lib/udev/rules.d folder and add this command to the script

Did you test this with DSM7?
IMHO under DSM7 the installer will not allow to access the folder /usr/lib/udev/rules.d as it does not run as root.

@BenjV
Copy link

BenjV commented Jun 15, 2021

Yes I tested with DSM 7
And to run the crtl script as root you should use this privilege file.
This will ensure that all the installation scripts will be run as root and the package itself not.

{
	"defaults": {
		"run-as": "package"
	},
	"username": "sc-domoticz",
	"groupname": "sc-domoticz",
	"ctrl-script": [{
		"action": "preinst",
		"run-as": "root"
	},
	{
		"action": "postinst",
		"run-as": "root"
	},
	{
		"action": "preuninst",
		"run-as": "root"
	},
	{
		"action": "postuninst",
		"run-as": "root"
	},
	{
		"action": "preupgrade",
		"run-as": "root"
	},
	{
		"action": "postupgrade",
		"run-as": "root"
	}]
}

See page 86 of the DSM 7 development guide.

To my opinion this privilege file should be used for all packages, because lots of them need root privilege to correctly be installed.
A lot of the issues mentioned in pr #4524 like the ones below will work again
Installer uses unsupported function like set_unix_permissions, syno_remove_user, syno_group_create, syno_group_remove, syno_user_add_to_group, set_syno_permissions, syno_user_add_to_legacy_group

And these lines should be added to the service-setup script.

echo 'KERNEL=="ttyUSB*", ACTION=="add", USER=${EFF_USER}, MODE="0600"' > /usr/lib/udev/rules.d/70-domotica.rules
echo 'KERNEL=="ttyACM*", ACTION=="add", USER=${EFF_USER}, MODE="0600"' >> /usr/lib/udev/rules.d/70-domotica.rules
udevadm control --reload-rules

I don't have knowledge of SynoCommunity framework only of how packages work so the translation of were to put it in the framework I don't know, I only see the end result (the .spk)

@hgy59
Copy link
Contributor Author

hgy59 commented Jun 15, 2021

Yes I tested with DSM 7
And to run the crtl script as root you should use this privilege file.
This will ensure that all the installation scripts will be run as root and the package itself not.

The problem is, that DSM7 does not accept any ctrl script entry

	"ctrl-script": [{
		"action": "preinst",
		"run-as": "package"
	},

Even with only one entry like the one above, (or an empty ctrl-script array) the DSM7 installer fails with the Message, that the package requires root privileges.

@publicarray can you report this error to synology please?

@BenjV
Copy link

BenjV commented Jun 15, 2021

Very strange, I checked several packages from Synology themselves and the all have ctrl_script settings in the privilege file.

@hgy59
Copy link
Contributor Author

hgy59 commented Jun 15, 2021

Very strange, I checked several packages from Synology themselves and the all have ctrl_script settings in the privilege file.

DSM7 packages from synology are encrypted (or signed?) so such packages are handled differently than thirdparty packages not published by synology.
As regarding the documentation this should be supported, I suppose this is really a bug in the DSM7 installer.

@BenjV
Copy link

BenjV commented Jun 15, 2021

I found this statement from Synology.
https://kb.synology.com/en-global/DSM/tutorial/supported_third_party_packages_beta

So it seems they only allow package signed by Synology to run as root.
I don't know if it was intended that this was extended to the ctrl scripts but is is at the moment although the development documentation states is should be supported as @hgy59 says.

If Synology does not change this, then the only solution I see is to add a bash script to the package which the user has to run from the commandline with correct bprivs.
Synology is on the way to become a closed environment, so it will no surprise to me that their next step would be to remove ssh from DSM.

- install/uninstall udev rules for usb devices (DSM<=6 only)
- configure log level
@hgy59
Copy link
Contributor Author

hgy59 commented Jun 15, 2021

@BenjV just updated the service installation

  • udev rules are now installed/uninstalled (for DSM<6 only)
  • the log level is now set to error,status (you requested error only)

As I do not have devices available that can send data to domoticz, can you please validate that measurement values are kept away from the log with the current settings (I suppose that one of the debuglevels hardware,received,eventsystem would add measurement value logs).

@BenjV
Copy link

BenjV commented Jun 16, 2021

@hgy59
There are no artifacts for one of your newer build.
So I cannot download a package to test.

@hgy59
Copy link
Contributor Author

hgy59 commented Jun 16, 2021

@hgy59
There are no artifacts for one of your newer build.
So I cannot download a package to test.

I can build one for you. What arch do you need?

@BenjV
Copy link

BenjV commented Jun 16, 2021

DS116 armada_38x

@hgy59
Copy link
Contributor Author

hgy59 commented Jun 16, 2021

x64 and armv7 packages are available here: https://github.com/hgy59/spksrc/releases/tag/domoticz_2020.2

@BenjV
Copy link

BenjV commented Jun 17, 2021

@hgy59
This package is working fine on DSM 7
With the current loglevel no data measurements are added to the logfile.

@hgy59 hgy59 merged commit 8c147c3 into SynoCommunity:master Jun 19, 2021
@hgy59 hgy59 deleted the update_domoticz branch June 19, 2021 18:33
@hgy59 hgy59 added the status/published Published and activated (may take up to 48h until visible in DSM package manager) label Jun 20, 2021
@hgy59 hgy59 mentioned this pull request Jun 26, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/published Published and activated (may take up to 48h until visible in DSM package manager) update request to update existing package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to install Domoticz - status Stopped
3 participants