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

Modularize Commotion Helpers #30

Closed
seamustuohy opened this issue Oct 14, 2013 · 13 comments
Closed

Modularize Commotion Helpers #30

seamustuohy opened this issue Oct 14, 2013 · 13 comments
Assignees

Comments

@seamustuohy
Copy link
Collaborator

Commotion Helpers (luasrc/commotion_helpers.lua) needs to be made into a module so that other wireless and mesh projects can choose only the subset of commotion helpers with the package that they want. This will also follow the existing standards used in the LuCI system.

See:
https://lists.chambana.net/pipermail/commotion-dev/2013-September/001604.html
http://www.lua.org/manual/5.2/manual.html#6.3
http://lua-users.org/wiki/ModulesTutorial (A short overview.)

@ghost ghost assigned seamustuohy Oct 14, 2013
@westbywest
Copy link
Collaborator

I'd love to try this out on WasabiNet, which is already bundling Commotion-OpenWRT's fork of olsrd and its luci theme.** How may I help?

A possibly relevant, unresolved detail may be how Commotion-OpenWRT and/or its constituent modules could be made to work with alternate captive portals. Genevieve has made progress with teasing commotion-splash (and thus nodog) out as a standalone module, but I can confirm many mesh networks use coovachilli (including WasabiNet) for their portal. Unlike commotion-splash, coovachilli is largely self-contained, not dependent on lua or a separate http server.

So, could this effort to modularize portions of Commotion also work with firmware platforms that do not use Luci/lua for the captive portal?

** W/r/t to bundling portions of Commotion codebase, what sort of permission or attribution would you prefer from mesh network operators?

@dismantl
Copy link
Contributor

just for clarification, nothing is dependent upon commotion-splash, so it is totally swappable w/ other captive portals for custom firmware images. Nodogsplash is also an independent package; commotion-splash adds additional functionality to NDS, which depends upon LuCI/Lua.

@jheretic
Copy link
Member

Ben, you might be interested in the recent repository refactoring work. As of right now, all packages requiring GUI components should be completely separate from all other Commotion OpenWRT packages, and there are much clearer dependencies and organization (this is if you pull from master). We'll need to do more testing, but it's a good first step for the modularization you're talking about.

@seamustuohy
Copy link
Collaborator Author

The new modular commotion helpers packages have been created. I am leaving testing instructions for individual package maintainers here so that I can track all testing before submitting and accepting all pull requests related to this at the same time.

The following repo's (issue_30 branch) need to be tested:

commotion-dashboard-helper - Nat/Andrew?
luci-commotion-quickstart - Seamus
luci-theme-commotion - Seamus/Dan/Andrew?
luci-commotion-apps - Dan
luci-commotion-splash - Dan
luci-commotion - Seamus/Dan/Andrew?

TO TEST:
Download the commotion-openwrt repo and change the commotion-feed repo in feeds.conf to the following

src-git commotion git://github.com/opentechinstitute/commotion-feed.git;issue_30_test

All repo's will be compiled on their current issue_30 test branch. From there you can look at the diff of your individual test branch and devise a test to cause the code to execute.

This can be an oppourtunity to create scripted unit-tests for your luci-applications. For instructions on how to build a command line script to easily recreate a luci submission please look at https://github.com/elationfoundation/luci_tutorials/blob/master/06-debugging.md#recreate-and-get-lua-error-debugging-output-on-the-command-line-

You can then ensure that the proper line of code was called by following the instructions at https://github.com/elationfoundation/luci_tutorials/blob/master/06-debugging.md#line-by-line-call-trace to get a full line trace of your code. This will allow you to ensure that you have tested each componenet and that it will be easy for you to reporduce this condition in your code if you need to in the future.

Once you have tested your pull request, please comment on this issue with the repo tested. Once every repo has been tested I will do accept all the pull requests at one time, and if you have created scripted tests (please do) I will then run all scripted tests to ensure that all pull requests together do not create any unintentional bugs.

Then we will celebrate. It will be a glorious festival of music and cheering. There will be drums, and singers, and stories of our epic accomplishments... or we will just add a release note and be on our way.

@seamustuohy
Copy link
Collaborator Author

Note: After running the scripted unit tests I have found them somewhat lacking for the purposes I laid out. Please feel free to use your own testing methods. Also, it should be noted that the line trace tool only reports function calls. So, you will have to trace back from the function calls in a function to see if a variable (like a=requires "whatever") was set.

@seamustuohy
Copy link
Collaborator Author

Pull Request #53: Commotion-Lua-Helpers https://github.com/opentechinstitute/luci-commotion-apps/issues/26
Testing complete and ready to be merged.

@seamustuohy
Copy link
Collaborator Author

Pull Request 2: Commotion-Lua-Helpers https://github.com/opentechinstitute/luci-theme-commotion/issues/2
Changes tested. Ready to be merged.

@seamustuohy
Copy link
Collaborator Author

Pull Request 28: Issue 30 opentechinstitute/commotion-feed#28
Changes tested.
Menuconfig is only selecting some of the packages as chosen (*), instead of required(--), even though they are required but they are chosen none-the-less. Ready to be merged.

@dismantl
Copy link
Contributor

dismantl commented Nov 5, 2013

luci-commotion-apps changes confirmed working.

@seamustuohy
Copy link
Collaborator Author

Commotion Splash has been tested. Changes confirmed, And one extra bug fix implemented. :)

@seamustuohy
Copy link
Collaborator Author

luci-commotion-dashboard Tested,

@seamustuohy
Copy link
Collaborator Author

Mic Drop!

@seamustuohy
Copy link
Collaborator Author

As epic as it would have been to have that last commit be the last commit on this issue...

Please see the merges from the issue_30 branches to master in the following repo's. This was then built and tested using the defaults on the build server.

commotion-dashboard-helper
luci-commotion-quickstart
luci-theme-commotion
luci-commotion-apps
luci-commotion-splash
luci-commotion
commotion-feed
commotion-openwrt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants