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.

Sign up for GitHub

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

Jump to bottom

luci-app-lorawan-basicstation: Add luci-app-lorawan-basicstation #5938

Merged
merged 1 commit into from
Sep 11, 2022
Merged

luci-app-lorawan-basicstation: Add luci-app-lorawan-basicstation #5938

merged 1 commit into from
Sep 11, 2022

Conversation

mars642
Copy link
Contributor

@mars642 mars642 commented Aug 18, 2022

LuCi support for Basicstation

Depends on openwrt/packages#19188

Signed-off-by: Marcus Schref mschref@web.de

@mars642
Copy link
Contributor Author

mars642 commented Aug 18, 2022

ping @aparcar

@aparcar
Copy link
Member

aparcar commented Aug 18, 2022

Please post some screenshots

@mars642
Copy link
Contributor Author

mars642 commented Aug 18, 2022

Here are some screenshots of the luci app
GeneralSettings1
GeneralSettings2
AdvancedSettings
LogMessages

@hnyman hnyman added the depends on other pr pull request depends on another pull request label Aug 18, 2022
@feckert feckert requested a review from aparcar August 18, 2022 10:50
BKPepe
BKPepe reviewed Aug 20, 2022
View reviewed changes
applications/luci-app-basicstation/Makefile Outdated
include $(TOPDIR)/rules.mk

PKG_NAME:=luci-app-basicstation
PKG_VERSION:=2.0.6
Copy link
Member

Choose a reason for hiding this comment

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

Why such versioning? Couldn't it be 1.0.0 as this is the first initial version, which could gets into this repository?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have completely removed the PKG_VERSION because it doesn't seem to be that common in LuCi packages. Do you agree with it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing versioning is fine, as there is automatic versioning in LuCI. (most packages do not have manual versions at all)

@hnyman
Copy link
Contributor

hnyman commented Aug 23, 2022

I would rename this to something like luci-app-lorawan-basicstation
And also add LoRaWAN to the title seen in menuconfig:
LUCI_TITLE:=LuCI Support for basicstation
--> LUCI_TITLE:=LuCI Support for LoRaWAN basicstation

Otherwise the "basic station" may sound too much like a simplified setup tool for a router. (there is already a PR for luci-mod-simple, etc.)

@mars642
luci-app-lorawan-basicstation: initial check-in
7b8cae3 
LuCi Support for LoRaWAN basicstation

Signed-off-by: Marcus Schref <mschref@web.de>
@mars642 mars642 requested review from BKPepe and removed request for aparcar August 23, 2022 12:45
@mars642 mars642 changed the title luci-app-basicstation: Add luci-app-basicstation luci-app-lorawan-basicstation: Add luci-app-lorawan-basicstation Aug 23, 2022
@hnyman hnyman merged commit 7bc7c8a into openwrt:master Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hnyman hnyman hnyman left review comments

@BKPepe BKPepe Awaiting requested review from BKPepe

Assignees
No one assigned
Labels
depends on other pr pull request depends on another pull request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mars642 @aparcar @hnyman @BKPepe