wlanslovenija / nodewatcher

A modular open networks growing platform.
http://nodewatcher.net/
Other
63 stars 20 forks source link

Add Xiaomi router Mini and Nano device descriptors #41

Closed robimarko closed 7 years ago

robimarko commented 7 years ago

Hi, another Pull request for adding new device descriptors. This time support for Xiaomi router Mini and Nano. Really good budget devices that outperform TP-Link at budget.

kostko commented 7 years ago

Please rebase on development.

kostko commented 7 years ago

Also, all commits related to device descriptors should have the devices: prefix.

robimarko commented 7 years ago

Ok,will rebase and follow devices: prefix

robimarko commented 7 years ago

@kostko Rebased.

kostko commented 7 years ago

Do I see correctly that this requires two new architectures to be built (ramips_mt7620 and ramips_mt7628)? We currently only build ramips_mt7621.

Also, you didn't update the commit message to include the devices: prefix. Once you do that I think it will be good to merge.

robimarko commented 7 years ago

Yes, Mini is ramips_mt7620 and Nano is ramips_mt7628 More and more devices are using ramips architecture. Prefix has been added

kostko commented 7 years ago

Prefix has been added

You should include the space as well, so devices: (space at the end, it seems GitHub removes it) :-)

robimarko commented 7 years ago

@kostko Space added. Yeah github removes it when pasting.

kostko commented 7 years ago

Please rebase on development.

mitar commented 7 years ago

@kostko: You can rebase while merging on GitHub? I do not think PR author should be doing that. They should be just making sure code is correct, and then we can decide at merging how we want to merge, with which commit message, etc.

robimarko commented 7 years ago

Well no issues,I can rebase now.

kostko commented 7 years ago

@kostko: You can rebase while merging on GitHub? I do not think PR author should be doing that. They should be just making sure code is correct, and then we can decide at merging how we want to merge, with which commit message, etc.

I always merge using git directly, not on GitHub as I want to be sure what is going on. And if I rebase there, then GitHub will not automatically close the PR. But sure, I can always do that myself and just rewrite the commits.

robimarko commented 7 years ago

Rebased.

mitar commented 7 years ago

I always merge using git directly, not on GitHub as I want to be sure what is going on.

Aha. I think they improved this. Now it is pretty clear what they are doing. But I understand. In that case request authors to rebase.

Sadly there is only "update branch" button on GitHub, which merges target branch in. And not "rebase branch".

robimarko commented 7 years ago

Changes made and Nano is no longer extending Minis class