Skip to contentSkip to conversation

package/mxt-app: new package

PATCH In review [email protected] 2 participants 3 messages Sep 19, 2023

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 package/mxt-app: new package

From: Saeed Kazemi <[email protected]>

mxt-app is a utility for managing Atmel maXTouch touch controllers and other devices that support Atmel Object Based Protocol.

---
 package/Config.in            |  1 +
 package/mxt-app/Config.in    |  8 ++++++++
 package/mxt-app/mxt-app.hash |  2 ++
 package/mxt-app/mxt-app.mk   | 20 ++++++++++++++++++++
 4 files changed, 31 insertions(+)
 create mode 100644 package/mxt-app/Config.in
 create mode 100644 package/mxt-app/mxt-app.hash
 create mode 100644 package/mxt-app/mxt-app.mk
package/Config.in +1 -0
index e8dbadadf3..e8d39b74b8 100644
@@ -556,6 +556,7 @@ endmenu
 	source "package/mhz/Config.in"
 	source "package/minicom/Config.in"
 	source "package/msr-tools/Config.in"
+	source "package/mxt-app/Config.in"
 	source "package/nanocom/Config.in"
 	source "package/neard/Config.in"
 	source "package/nvidia-driver/Config.in"
package/mxt-app/Config.in new file +8 -0
new file mode 100644
index 0000000000..7e8a5f4901
@@ -0,0 +1,8 @@
+config BR2_PACKAGE_MXT_APP
+	bool "mxt-app"
+	select BR2_PACKAGE_LIBUSB
+	help
+	  mxt-app is a utility for managing Atmel maXTouch touch controllers
+	  and other devices that support Atmel Object Based Protocol.
+
+	  https://github.com/atmel-maxtouch/mxt-app/
package/mxt-app/mxt-app.hash new file +2 -0
new file mode 100644
index 0000000000..abf20c728a
@@ -0,0 +1,2 @@
+# Locally calculated
+sha256  3639720ae1ea033bd982d68c5cab84432ce046fe14a66d9b2de07f115b642722  v1.36.tar.gz
package/mxt-app/mxt-app.mk new file +20 -0
new file mode 100644
index 0000000000..626cd352fc
@@ -0,0 +1,20 @@
+################################################################################
+#
+# mxt-app
+#
+################################################################################
+
+MXT_APP_VERSION = 1.36
+MXT_APP_SOURCE = v$(MXT_APP_VERSION).tar.gz
+MXT_APP_SITE = https://github.com/atmel-maxtouch/mxt-app/archive/refs/tags
+MXT_APP_LICENSE = Atmel License
+MXT_APP_LICENSE_FILES = LICENSE
+MXT_APP_DEPENDENCIES = libusb
+MXT_APP_INSTALL_STAGING = YES
+MXT_APP_AUTORECONF = YES
+MXT_APP_CONF_OPTS = --enable-debug
+
+MXT_APP_CXXFLAGS = $(TARGET_CXXFLAGS)
+MXT_APP_CONF_ENV = CXXFLAGS="$(MXT_APP_CXXFLAGS)"
+
+$(eval $(autotools-package))
Signature · 6 lines
2.34.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#1 lore raw

Hello,

Quoted 6 lines · On Tue, 19 Sep 2023 14:55:42 +0200 [email protected] wrote:

From: Saeed Kazemi <[email protected]>

mxt-app is a utility for managing Atmel maXTouch touch controllers and other devices that support Atmel Object Based Protocol.

Thanks for this contribution! This looks pretty good, but I have a few comments below.

Quoted 9 lines
---
 package/Config.in            |  1 +
 package/mxt-app/Config.in    |  8 ++++++++
 package/mxt-app/mxt-app.hash |  2 ++
 package/mxt-app/mxt-app.mk   | 20 ++++++++++++++++++++
 4 files changed, 31 insertions(+)
 create mode 100644 package/mxt-app/Config.in
 create mode 100644 package/mxt-app/mxt-app.hash
 create mode 100644 package/mxt-app/mxt-app.mk

Could you add an entry in the DEVELOPERS file for this package?

package/mxt-app/Config.in line 3

Quoted 9 lines
package/mxt-app/Config.in new file +3 -0
new file mode 100644
index 0000000000..7e8a5f4901
@@ -0,0 +1,8 @@
+config BR2_PACKAGE_MXT_APP
+	bool "mxt-app"
+	select BR2_PACKAGE_LIBUSB

You need to replicate the "depends on" from libusb:

depends on BR2_TOOLCHAIN_HAS_THREADS # libusb depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 # libusb

package/mxt-app/Config.in line 8

Quoted 5 lines
package/mxt-app/Config.in new file +5 -0
+	help
+	  mxt-app is a utility for managing Atmel maXTouch touch controllers
+	  and other devices that support Atmel Object Based Protocol.
+
+	  https://github.com/atmel-maxtouch/mxt-app/

And add a Config.in comment:

comment "mxt-app needs a toolchain w/ threads, gcc >= 4.9"

        depends on !BR2_TOOLCHAIN_HAS_THREADS || \
                !BR2_TOOLCHAIN_GCC_AT_LEAST_4_9

package/mxt-app/mxt-app.hash line 2

Quoted 8 lines
package/mxt-app/mxt-app.hash new file +2 -0
new file mode 100644
index 0000000000..abf20c728a
@@ -0,0 +1,2 @@
+# Locally calculated
+sha256  3639720ae1ea033bd982d68c5cab84432ce046fe14a66d9b2de07f115b642722  v1.36.tar.gz

We need the hash of the license file to be present.

Quoted 6 lines
package/mxt-app/mxt-app.mk new file +0 -0
new file mode 100644
index 0000000000..626cd352fc

package/mxt-app/mxt-app.mk line 9

Quoted 9 lines
package/mxt-app/mxt-app.mk new file +9 -0
@@ -0,0 +1,20 @@
+################################################################################
+#
+# mxt-app
+#
+################################################################################
+
+MXT_APP_VERSION = 1.36
+MXT_APP_SOURCE = v$(MXT_APP_VERSION).tar.gz
+MXT_APP_SITE = https://github.com/atmel-maxtouch/mxt-app/archive/refs/tags

Could you use the github macro instead?

MXT_APP_VERSION = 1.36
MXT_APP_SITE = $(call github,atmel-maxtouch,mxt-app,v$(MXT_APP_VERSION))

you will to adjust the hash file accordingly.

package/mxt-app/mxt-app.mk line 10

Quoted 1 line
package/mxt-app/mxt-app.mk new file +1 -0
+MXT_APP_LICENSE = Atmel License

The license is actually BSD-2-Clause

package/mxt-app/mxt-app.mk line 15

Quoted 5 lines
package/mxt-app/mxt-app.mk new file +5 -0
+MXT_APP_LICENSE_FILES = LICENSE
+MXT_APP_DEPENDENCIES = libusb
+MXT_APP_INSTALL_STAGING = YES
+MXT_APP_AUTORECONF = YES
+MXT_APP_CONF_OPTS = --enable-debug

Why --enable-debug by default?

package/mxt-app/mxt-app.mk line 18

Quoted 2 lines
package/mxt-app/mxt-app.mk new file +2 -0
+MXT_APP_CXXFLAGS = $(TARGET_CXXFLAGS)
+MXT_APP_CONF_ENV = CXXFLAGS="$(MXT_APP_CXXFLAGS)"

I'm not sure what's the purpose of those two lines. CXXFLAGS are already passed by the autotools-package infrastructure in the environment of the configure script, so this seems redundant.

Could you rework the patch to take into account those suggestions, and send an updated version?

Also, make sure to:

(1) Run "make check-package" to validate the coding style

(2) Use utils/test-pkg to validate that the package builds fine

Thanks a lot!

Thomas

Signature · 7 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#2 lore raw

Hello,
Thanks for your feedback.

I applied your suggestions and also added an option for enabling debug (disable by default)

Thanks,
Saeed

Quoted 124 lines · On Tue, Sep 19, 2023 at 15:19 Thomas Petazzoni <[email protected]>…

Hello,

Quoted 6 lines · On Tue, 19 Sep 2023 14:55:42 +0200 [email protected] wrote:

From: Saeed Kazemi <[email protected]>

mxt-app is a utility for managing Atmel maXTouch touch controllers and other devices that support Atmel Object Based Protocol.

Thanks for this contribution! This looks pretty good, but I have a few comments below.

Quoted 9 lines
---
 package/Config.in            |  1 +
 package/mxt-app/Config.in    |  8 ++++++++
 package/mxt-app/mxt-app.hash |  2 ++
 package/mxt-app/mxt-app.mk   | 20 ++++++++++++++++++++
 4 files changed, 31 insertions(+)
 create mode 100644 package/mxt-app/Config.in
 create mode 100644 package/mxt-app/mxt-app.hash
 create mode 100644 package/mxt-app/mxt-app.mk

Could you add an entry in the DEVELOPERS file for this package?

package/mxt-app/Config.in line 3

Quoted 9 lines
package/mxt-app/Config.in new file +3 -0
new file mode 100644
index 0000000000..7e8a5f4901
@@ -0,0 +1,8 @@
+config BR2_PACKAGE_MXT_APP
+     bool "mxt-app"
+     select BR2_PACKAGE_LIBUSB

You need to replicate the "depends on" from libusb:

depends on BR2_TOOLCHAIN_HAS_THREADS # libusb depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 # libusb

package/mxt-app/Config.in line 5

Quoted 2 lines
package/mxt-app/Config.in new file +2 -0
+     help
+       mxt-app is a utility for managing Atmel maXTouch touch

controllers

package/mxt-app/Config.in line 8

Quoted 3 lines
package/mxt-app/Config.in new file +3 -0
+       and other devices that support Atmel Object Based Protocol.
+
+       https://github.com/atmel-maxtouch/mxt-app/

And add a Config.in comment:

comment "mxt-app needs a toolchain w/ threads, gcc >= 4.9"

        depends on !BR2_TOOLCHAIN_HAS_THREADS || \
                !BR2_TOOLCHAIN_GCC_AT_LEAST_4_9

package/mxt-app/mxt-app.hash line 2

Quoted 8 lines
package/mxt-app/mxt-app.hash new file +2 -0
new file mode 100644
index 0000000000..abf20c728a
@@ -0,0 +1,2 @@
+# Locally calculated
+sha256

3639720ae1ea033bd982d68c5cab84432ce046fe14a66d9b2de07f115b642722 v1.36.tar.gz

We need the hash of the license file to be present.

package/mxt-app/mxt-app.mk

Quoted 7 lines
package/mxt-app/mxt-app.mk new file +0 -0
new file mode 100644
index 0000000000..626cd352fc
@@ -0,0 +1,20 @@

+################################################################################

package/mxt-app/mxt-app.mk line 4

Quoted 4 lines
package/mxt-app/mxt-app.mk new file +3 -0
+#
+# mxt-app
+#

+################################################################################

package/mxt-app/mxt-app.mk

Quoted 4 lines
package/mxt-app/mxt-app.mk new file +4 -0
+
+MXT_APP_VERSION = 1.36
+MXT_APP_SOURCE = v$(MXT_APP_VERSION).tar.gz
+MXT_APP_SITE =

https://github.com/atmel-maxtouch/mxt-app/archive/refs/tags

Could you use the github macro instead?

MXT_APP_VERSION = 1.36
MXT_APP_SITE = $(call github,atmel-maxtouch,mxt-app,v$(MXT_APP_VERSION))

you will to adjust the hash file accordingly.

package/mxt-app/mxt-app.mk line 10

Quoted 1 line
package/mxt-app/mxt-app.mk new file +1 -0
+MXT_APP_LICENSE = Atmel License

The license is actually BSD-2-Clause

package/mxt-app/mxt-app.mk line 15

Quoted 5 lines
package/mxt-app/mxt-app.mk new file +5 -0
+MXT_APP_LICENSE_FILES = LICENSE
+MXT_APP_DEPENDENCIES = libusb
+MXT_APP_INSTALL_STAGING = YES
+MXT_APP_AUTORECONF = YES
+MXT_APP_CONF_OPTS = --enable-debug

Why --enable-debug by default?

package/mxt-app/mxt-app.mk line 18

Quoted 2 lines
package/mxt-app/mxt-app.mk new file +2 -0
+MXT_APP_CXXFLAGS = $(TARGET_CXXFLAGS)
+MXT_APP_CONF_ENV = CXXFLAGS="$(MXT_APP_CXXFLAGS)"

I'm not sure what's the purpose of those two lines. CXXFLAGS are already passed by the autotools-package infrastructure in the environment of the configure script, so this seems redundant.

Could you rework the patch to take into account those suggestions, and send an updated version?

Also, make sure to:

(1) Run "make check-package" to validate the coding style

(2) Use utils/test-pkg to validate that the package builds fine

Quoted 8 lines

Thanks a lot!

Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin Embedded Linux and Kernel engineering and training
https://bootlin.com

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot