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

INFO and config alignment spksrc.service.mk #4993

Merged
merged 2 commits into from
Dec 8, 2021
Merged

INFO and config alignment spksrc.service.mk #4993

merged 2 commits into from
Dec 8, 2021

Conversation

BKSteve
Copy link
Contributor

@BKSteve BKSteve commented Dec 6, 2021

Create config file with same id as INFO file.

Motivation: DSM7 notification not working as INFO file of package and config file are not aligned in naming convention.

Linked issues: https://github.com/SickChill/SickChill/issues/7694#issuecomment-986926967

Thus synodsmnotify even if strings were configured correctly would not resolve their i18n text strings.

@th0ma7 I don't know the knock on effects of this modification to other parts of the build. Also the other choice may be to modify the spksrc.spk.mk file lines 204 to 206 to change the INFO file instead.

Create config file with same id as INFO file.
@hgy59
Copy link
Contributor

hgy59 commented Dec 6, 2021

The following code in spksrc.spk.mk must be updated

ifneq ($(strip $(DSM_APP_NAME)),)
	@echo dsmappname=\"$(DSM_APP_NAME)\" >> $@
	@echo dsmapppage=\"$(DSM_APP_NAME)\" >> $@
	@echo dsmapplaunchname=\"$(DSM_APP_NAME)\" >> $@
else
	@echo dsmappname=\"com.synocommunity.$(SPK_NAME)\" >> $@
	@echo dsmapppage=\"com.synocommunity.$(SPK_NAME)\" >> $@
	@echo dsmapplaunchname=\"com.synocommunity.$(SPK_NAME)\" >> $@
endif
  1. The prefix must be com.synocommunity.packages.
  2. dsmapppage must not be the same as dsmappname
  3. dsmapplaunchname is optional and must only be defined if different than dsmappname
  4. dsmapppage and dsmapplaunchname are valid for DSM 7 only

So it should look something like this:

ifneq ($(strip $(DSM_APP_NAME)),)
	@echo dsmappname=\"$(DSM_APP_NAME)\" >> $@
else
	@echo dsmappname=\"com.synocommunity.packages.$(SPK_NAME)\" >> $@
endif
ifeq ($(call version_ge, ${TCVERSION}, 7.0),1)
ifneq ($(strip $(DSM_APP_PAGE)),)
	@echo dsmapppage=\"$(DSM_APP_PAGE)\" >> $@
endif
ifneq ($(strip $(DSM_APP_LAUNCH_NAME)),)
	@echo dsmapplaunchname=\"$(DSM_APP_LAUNCH_NAME)\" >> $@
endif
endif

@hgy59 hgy59 added the framework label Dec 6, 2021
@hgy59
Copy link
Contributor

hgy59 commented Dec 6, 2021

ifneq ($(strip $(DSM_APP_NAME)),)
@echo dsmappname="$(DSM_APP_NAME)" >> $@
else
@echo dsmappname="com.synocommunity.packages.$(SPK_NAME)" >> $@
endif
ifeq ($(call version_ge, ${TCVERSION}, 7.0),1)
ifneq ($(strip $(DSM_APP_PAGE)),)
@echo dsmapppage="$(DSM_APP_PAGE)" >> $@
endif
ifneq ($(strip $(DSM_APP_LAUNCH_NAME)),)
@echo dsmapplaunchname="$(DSM_APP_LAUNCH_NAME)" >> $@
endif
endif

BTW this brings back the DSM desktop icons for synocommunity apps in DSM 6 and DSM 7.

@BKSteve can you please explain what the synodsmnotify means and how we can test it.

- fix dsmappname in spksrc.spk.mk and id in app/config generated in spksrc.service.mk (add .packages)
- add Makefile variable DSM_APP_PAGE to define dsmapppage in DSM 7 INFO file
- add Makefile variable DSM_APP_LAUNCH_NAME to define dsmapplaunchname in DSM 7 INFO file
@@ -39,7 +39,7 @@
# folder at intallation and runtime.
# SSS_SCRIPT (optional) custom script file for service start/stop/status when the generic
# installer generated script (SERVICE_SETUP) is not usable.
# NO_SERVICE_SHORTCUT (optional) do not create
# NO_SERVICE_SHORTCUT (optional) do not create an app icon in the DSM desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just an update of the comment and has to be validated for correct implementation.

@hgy59
Copy link
Contributor

hgy59 commented Dec 6, 2021

@BKSteve can you please explain what the synodsmnotify means and how we can test it.

Just found it in the Dev Guide (page 59):

Show Message as Desktop Notification

It is possible to use /usr/syno/bin/synodsmnotify executable to send desktop notifications to users. The notification title / message
must be an I18N string.

/usr/syno/bin/synodsmnotify -c [app_id] [user_or_group] [i18n_string_for_title] [i18n_string_for_msg]
/usr/syno/bin/synodsmnotify -c com.company.App1 admin MyPackage:app_tree:index_title MyPackage:app_tree:node_1
/usr/syno/bin/synodsmnotify -c com.company.App1 @administrators MyPackage:app_tree:index_title MyPackage:app_tree:node_1

Notification title and message here should be in the format of [package_id]:[i18n_section]:[i18n_key] where package_id is the package value in package INFO file. I18N string example can be found in I18N page. Remember to specify desktop notification strings to preloadTexts field in application config.

@BKSteve
Copy link
Contributor Author

BKSteve commented Dec 7, 2021

BTW this brings back the DSM desktop icons for synocommunity apps in DSM 6 and DSM 7.

Yes, when the two files config and INFO align they will work together.

the other choice may be to modify the spksrc.spk.mk file lines 204 to 206 to change the INFO file instead.

So from the edits that clears up that question / comment.

I have another question, is it necessary to do
ifeq ($(call version_ge, ${TCVERSION}, 7.0),1)
as I thought both DSM6 and DSM7 would still need the dsmapppage and dsmapplaunchname but I can't see where these variables are set for DSM6.

Does this also mean that DSM_APP_PAGE and DSM_APP_LAUNCH_NAME need to be defined elsewhere? These variables aren't in any other location that I can find.

Add1:
On the text from page 59 of the manual.
You don't need the -c com.company.App1 in the string
/usr/syno/bin/synodsmnotify -c com.company.App1 @administrators MyPackage:app_tree:index_title MyPackage:app_tree:node_1
so to trigger notification this works
/usr/syno/bin/synodsmnotify @administrators MyPackage:app_tree:index_title MyPackage:app_tree:node_1

Add2:
You may have seen me asking how to get some strings into the config file on Discord but for SickChill this isn't important as it's only used (presently) for when running notifications and not starting up or shutdown notifications.
My config manually edited with "preloadTexts": [ "appsc:apptitle", "appsc:action" ] done for testion

{
  ".url": {
    "com.synocommunity.packages.sickchill": {
      "type": "url",
      "title": "SickChill",
      "desc": "Automatic Video Library Manager for TV Shows. It watches for new episodes of your favorite shows, and when they are posted it does its magic.",
      "icon": "images/sickchill-{0}.png",
      "protocol": "http",
      "port": "8081",
      "url": "/",
      "allUsers": true,
      "grantPrivilege": "all",
      "advanceGrantPrivilege": true,
      "texts": "texts",
      "preloadTexts": [
        "appsc:apptitle",
        "appsc:action"
      ]
    }
  }
}

The "texts": "texts", may or may not be required as it was added whilst I was working with Synology on getting this to work.
Then /volume1/@appstore/sickchill/app/texts/enu/strings

[appsc]
action = "Action"
apptitle = "SickChill"

in line with the above the activation string for my testing was
/usr/syno/bin/synodsmnotify @administrators sickchill:appsc:apptitle sickchill:appsc:action
Which eventually worked once INFO and config were aligned, initially to com.synocommunity.sickchill but sure it'll work if all are com.synocommunity.packages.sickchill

@hgy59
Copy link
Contributor

hgy59 commented Dec 7, 2021

I have another question, is it necessary to do ifeq ($(call version_ge, ${TCVERSION}, 7.0),1) as I thought both DSM6 and DSM7 would still need the dsmapppage and dsmapplaunchname but I can't see where these variables are set for DSM6.

Does this also mean that DSM_APP_PAGE and DSM_APP_LAUNCH_NAME need to be defined elsewhere? These variables aren't in any other location that I can find.

Just from the dev guide (dsmapppage and dsmapplaunchname are new for DSM 7):

Field Name: dsmappname

  • Description: The value of each individual application will be equal to the unique property name in DSM’s config file so as to be
    integrated into Synology DiskStation.

    Note: Please refer Config in Integrate Your package into DSM chapter for more information.

  • Value: (Separated with a space)
  • Default Value: (Empty)
  • Example:
    dsmappname="SYNO.SDS.PhotoStation SYNO.SDS.PersonalPhotoStation"
    
  • DSM Requirement: 3.2-1922

Field Name: dsmapppage

  • Description: The application page to open when click on package open button (should be used with dsmappname key)
  • Value: Page name

    Note: page name corresponds to PageListAppWindow's fn value when calling SYNO.SDS.AppLaunch

  • Default Value: (Empty)
  • Example:
    dsmappname="SYNO.SDS.AdminCenter.Application"
    dsmapppage="SYNO.SDS.AdminCenter.FileService.Main"
    
  • DSM Requirement: 7.0-40332

Field Name: dsmapplaunchname

  • Description: The value will be used to launch desktop app, and it has higher priority than dsmappname.
  • Value: App name
  • Default Value: same as dsmappname
  • Example:
    dsmapplaunchname="SYNO.SDS.AdminCenter.Application"
    
  • DSM Requirement: 7.0-40796

As the values for dsmapppage and dsmapplaunchname are app specific, I introduced related Makefile variables. Those should never be the same as dsmappname.

Tie fields dsmappname, dsmapppage and dsmapplaunchname are optional in the INFO file and are only required for packages that need DSM desktop integration.
Packages that need such values in the INFO file must define DSM_APP_PAGE and/or DSM_APP_LAUNCH_NAME in the spk Makefile.
Plus those values must have matching definitions in the package app config file.

@BKSteve
Copy link
Contributor Author

BKSteve commented Dec 7, 2021

Here's my INFO file first 20 lines as installed in /var/packages/sickchill/ in my DSM7 which are the same as the INFO file in spk.
I've just checked my DSM6 and they existed in that INFO file too, all the same.

package="sickchill"
version="20211110-4"
description="Automatic Video Library Manager for TV Shows. It watches for new episodes of your favorite shows, and when they are posted it does its magic."

arch="apollolake avoton braswell broadwell broadwellnk bromolow cedarview denverton dockerx64 geminilake grantley purley kvmx64 v1000 x86 x86_64"
maintainer="miigotu"
maintainer_url=""
distributor=""
distributor_url=""
os_min_ver="7.0-41890" [ OR os_min_ver="6.1-15047"]
helpurl="https://sickchill.github.io/"
displayname="SickChill"
dsmuidir="app"
dsmappname="com.synocommunity.sickchill"
dsmapppage="com.synocommunity.sickchill"
dsmapplaunchname="com.synocommunity.sickchill"
adminport="8081"
changelog="1. SickChill environment fix<br/>2. Move to Python 3.8 without git<br/>3. Added DSM7 Support<br/>4. cryptography resolution for armv7"
install_dep_packages="python38>=3.8.12-6"
checksum="4383ccaaac0d2b013e4ec33f3ed3894c"

@th0ma7
Copy link
Contributor

th0ma7 commented Dec 7, 2021

Interestingly I noticed that tvheadend on DSM7 does not show an icon anymore... neither are other "web" applications. Is this related?

@BKSteve
Copy link
Contributor Author

BKSteve commented Dec 7, 2021

As the two files don't line up with each other then it can't match the information in the config file to what INFO is telling it to look for.
So yes most likely the cause.

After I set both files to contain com.synocommunity.packages.sickchill and restarted the icon was back. With only com.synocommunity.sickchill it was not there.

@hgy59
Copy link
Contributor

hgy59 commented Dec 7, 2021

I've just checked my DSM6 and they existed in that INFO file too, all the same.

Yes, that was what the spksrc framework generated, but the synology dev guide tells, that dsmapppage and dsmapplaunchname are not used in DSM 6.

@hgy59
Copy link
Contributor

hgy59 commented Dec 7, 2021

Interestingly I noticed that tvheadend on DSM7 does not show an icon anymore... neither are other "web" applications. Is this related?

Yes, that was my finding too, and therefore I jumped into this PR (was originally created to solve the DSM notifications from sickchill).

@BKSteve
Copy link
Contributor Author

BKSteve commented Dec 7, 2021

Yes, that was what the spksrc framework generated, but the synology dev guide tells, that dsmapppage and dsmapplaunchname are not used in DSM 6.

But why bother taking it out with if statements if it causes no issue?

I also don't fully understand how to use those 2 new items either

@th0ma7
Copy link
Contributor

th0ma7 commented Dec 7, 2021

My PR #4994 is close to ready for merge but I'm really much interested in having this icon thing fixed. I'll wait for this fix to get in before publishing my tvheadend update. I haven't played much with that portion of the code but let me know if I can be of assistance.

@BKSteve
Copy link
Contributor Author

BKSteve commented Dec 8, 2021

I will also leave it to the professionals. I've done my bit, identified and proposed a solution/s. Which will fix the initial issue and by chance another.

Leaving this to you guys in SynoComm now.

@hgy59 hgy59 merged commit fc1982b into SynoCommunity:master Dec 8, 2021
@BKSteve BKSteve deleted the INFO-config branch December 9, 2021 10:21
@th0ma7
Copy link
Contributor

th0ma7 commented Dec 13, 2021

@hgy59 and @BKSteve tested with tvheadend on DSM7 and its now working great! Good job!

publicarray pushed a commit to publicarray/spksrc that referenced this pull request Dec 19, 2021
* Update spksrc.service.mk
Create config file with same id as INFO file.
* keep com.synocommunity.packages in app name
- fix dsmappname in spksrc.spk.mk and id in app/config generated in spksrc.service.mk (add .packages)
- add Makefile variable DSM_APP_PAGE to define dsmapppage in DSM 7 INFO file
- add Makefile variable DSM_APP_LAUNCH_NAME to define dsmapplaunchname in DSM 7 INFO file

Co-authored-by: hgy59 <hpgy59@gmail.com>
AlexPresso pushed a commit to AlexPresso/spksrc that referenced this pull request Jan 30, 2025
* Update spksrc.service.mk
Create config file with same id as INFO file.
* keep com.synocommunity.packages in app name
- fix dsmappname in spksrc.spk.mk and id in app/config generated in spksrc.service.mk (add .packages)
- add Makefile variable DSM_APP_PAGE to define dsmapppage in DSM 7 INFO file
- add Makefile variable DSM_APP_LAUNCH_NAME to define dsmapplaunchname in DSM 7 INFO file

Co-authored-by: hgy59 <hpgy59@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants