Discover local IP on rov network#315
Conversation
cal-pratt
left a comment
There was a problem hiding this comment.
Looking good apart from some small fixes.
To edit a single commit to make changes you can use rebase interactive (yseterday we used the normal rebase command) https://stackoverflow.com/a/29950959/3280538
Use the awesome interactive rebase:
git rebase -i @~9 # Show the last 9 commits in a text editorFind the commit you want, change
picktoe(edit), and save and close the file. Git will rewind to that commit, allowing you to either:
- use
git commit --amendto make changes, or- use
git reset @~to discard the last commit, but not the changes to the files (i.e. take you to the point you were at when you'd edited the files, but hadn't committed yet).
I usually following this way to start my edits, but use different commands to edit. Go to the commit where you made changes by using git rebase -i @~4 and changing the tag on the commits you want to edit to edit, then save (this is vim; all the commands can be found on stackoverflow).
Make changes to the commits and then complete them by writing:
git commit --all --amend --no-edit
git rebase --continueAfter you do this, your local branch will be different from the origin branch, so you'll need to make sure everything still compiles before you --force push the branch to origin. You might need to run this a few times to get the hang of it, it's a bit tricky 🔨
You also need to remove the commit with files playbooks/files/config.yml and src/test/java/com/easternedgerobotics/rov/integration/AltIMU10v3Test.java. Those changes only help with your personal debugging and shouldn't be pushed to the master branch.
Deleting a git commit: https://www.clock.co.uk/insight/deleting-a-git-commit
Using Cherry Pick
Step 1: Find the commit before the commit you want to remove git log
Step 2: Checkout that commit git checkout
Step 3: Make a new branch using your current checkout commit git checkout -b
Step 4: Now you need to add the commit after the removed commit git cherry-pick
Step 5: Now repeat Step 4 for all other commits you want to keep.
Step 6: Once all commits have been added to your new branch and have been commited. Check that everything is in the correct state and working as intended. Double check everything has been commited: git status
Step 7: Switch to your broken branch git checkout
Step 8: Now perform a hard reset on the broken branch to the commit prior to the one your want to remove git reset --hard
Step 9: Merge your fixed branch into this branch git merge
Step 10: Push the merged changes back to origin. WARNING: This will overwrite the remote repo! git push --force origin
You can do the process without creating a new branch by replacing Step 2 & 3 with Step 8 then not carry out Step 7 & 9.
Example
Say we want to remove commits 2 & 4 from the repo.
git checkout b3d92c5 Checkout the last usable commit.
git checkout -b repair Create a new branch to work on.
git cherry-pick 77b9b82 Run through commit 3.
git cherry-pick 2c6a45b Run through commit 1.
git checkout master Checkout master.
git reset --hard b3d92c5 Reset master to last usable commit.
git merge repair Merge our new branch onto master.
git push --hard origin master Push master to the remote repo.
Final noteGit rebase & cherrypick are dangerous but powerful solutions that should only be used as a last option and only be undertaken by someone who knows what they are doing. Beware that both solutions could have adverse effects on other users who are working on the same repository / branch.
Finally remember to be careful and good luck!
| @@ -0,0 +1,2 @@ | |||
| launch: | |||
| broadcast: 192.168.56.255 No newline at end of file | |||
There was a problem hiding this comment.
Remove this file from the commit. It should be left empty.
| String.valueOf(t.getTemperature()))) | ||
| .subscribe(System.out::println); | ||
| eventPublisher.valuesOfType(VideoValueA.class).subscribe(System.out::println); | ||
|
|
There was a problem hiding this comment.
This file should not be edited.
| public VideoDecoder( | ||
| final EventPublisher eventPublisher, | ||
| final VideoDecoderConfig config | ||
| final VideoDecoderConfig config, final String broadcast |
There was a problem hiding this comment.
Should stay consistent with the rest of the params.
public VideoDecoder(
final EventPublisher eventPublisher,
final VideoDecoderConfig config,
final String broadcast
) {
...| final List<String> ipAddresses = new ArrayList<String>(); | ||
| try { | ||
| final Enumeration<NetworkInterface> network = NetworkInterface.getNetworkInterfaces(); | ||
| for (; network.hasMoreElements();) { |
There was a problem hiding this comment.
Use while loops instead of for loops here.
| final Enumeration<NetworkInterface> network = NetworkInterface.getNetworkInterfaces(); | ||
| for (; network.hasMoreElements();) { | ||
| final Enumeration<InetAddress> inet = network.nextElement().getInetAddresses(); | ||
| for (; inet.hasMoreElements();) { |
| } | ||
| } | ||
| if (!initialized) { | ||
| throw new IllegalStateException(); |
There was a problem hiding this comment.
After reviewing I actually think this would be better as a Logger#warn
if (!initialized) {
Logger.warn("Could not detect the IP of the runtime system");
}| } catch (final SocketException error) { | ||
| Logger.error(error); | ||
| } | ||
| for (String newAddress: ipAddresses) { |
There was a problem hiding this comment.
make newAddress final
for (final String newAddress : ipAddresses) {
| if (lastLocation >= 0) { | ||
| testAddress = newAddress.substring(1, lastLocation - 1); | ||
| } | ||
| if (broadcastIP.equals(testAddress)) { |
There was a problem hiding this comment.
I would just just nest the conditional to avoid the non-final testAddress variable.
final int lastLocation = newAddress.lastIndexOf('.');
if (lastLocation >= 0) {
final String testAddress = newAddress.substring(1, lastLocation - 1);
if (broadcastIP.equals(testAddress)) {
...
}
}
3da59e1 to
b111bd6
Compare
cal-pratt
left a comment
There was a problem hiding this comment.
Looks good apart from the one lib in build gradle.
| dependencies { | ||
| compile "com.fasterxml.jackson.module:jackson-module-kotlin:2.8.7" | ||
| compile files("${System.properties['java.home']}/../lib/tools.jar") | ||
| compile 'com.fasterxml.jackson.core:jackson-databind:2.5.3' |
There was a problem hiding this comment.
You kept the wrong line in the conflict resolution
we need:
compile "com.fasterxml.jackson.module:jackson-module-kotlin:2.8.7"
you kept the old lib:
compile 'com.fasterxml.jackson.core:jackson-databind:2.5.3'
In this pull request, I added a section that discovers all the IP's on the local computer and compare them to the broadcast IP from defaultConfig. If one is found that is a match, that IP will be broadcast to be able to receive video. If no matching IP is found, an IllegalStateException will be thrown.