Project

General

Profile

Actions

Bug #324

closed

(LEDE - batman-adv 2016.5) Setting aggregated_ogms in /etc/config/batman-adv not honoured; (parsing issue in /lib/batman-adv/config.sh)

Added by Edward Beech almost 8 years ago. Updated over 7 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Start date:
01/17/2017
Due date:
% Done:

0%

Estimated time:
1.00 h

Description

Hi folks,

I've never contributed before, so I apologise if I've done this wrong; I'm still not 100% sure if this issue should be against OpenWrt/LEDE or batman-adv.

I have noticed that for some time now, it's not possible to set aggregated_ogms setting from /etc/config/batman-adv.

Here's why:

First, we declare some locals
...
(line 14) local aggregated_ogms ap_isolation bonding bridge_loop_avoidance distributed_arp_table fragmentation
...

Then we try to read the variable from the file
...
(line 18) config_get aggregated_ogms "$mesh" aggregated_ogms
...

And finally we go to set the object in the filesystem
...
(line 37) [ -n "$aggregate_ogms" ] && echo $aggregate_ogms > /sys/class/net/$mesh/mesh/aggregate_ogms
...

However- you will note that there is a typo/oversight in line 37, the reference is "aggregate_ogms" instead of "aggregated_ogms"; a directory listing shows that it should indeed be "aggregated_ogms".

  1. ls /sys/class/net/bat0/mesh/agg*
    /sys/class/net/bat0/mesh/aggregated_ogms

Correcting the typo resolves the issue, I've tested this on one of my devices (GLi MiFi - LEDE build).

Regards,

Ed

Actions #1

Updated by Sven Eckelmann almost 8 years ago

  • Status changed from New to In Progress
  • Assignee set to Edward Beech

Nice find. But can you please submit a patch (as pull request) for this to https://github.com/openwrt-routing/packages . Please don't forget to increase the "PKG_RELEASE" in the batman-adv package Makefile.

You can try to trigger Simon and Marek to look at this pull requests by adding their user names (@simonwunderlich @lindnermarek) in a comment of the new pull request.

Actions #2

Updated by Edward Beech almost 8 years ago

Hi Sven,

Thanks for the followup; it'll be murky water for me, I've not submitted a patch for OpenWrt before.

I'll do a bunch of reading and try and knock it out correctly over the weekend.

Regards,

Ed

Actions #3

Updated by Sven Eckelmann almost 8 years ago

You basically have to fork the openwrt-routing repository on github, add the commit which fixes the problem and push it to your forked repository. Then you press "Pull Request" and fill out the form.

A good example is following pull request: https://github.com/openwrt-routing/packages/pull/221

You can see that the pull request message is basically the commit message. The commit message has:

  • "Signed-off-by:" from the author at the end (he basically used 'git commit -a -s' to create the commit)
  • used correct author name + email when creating the commit
  • used the prefix "batman-adv: " in the commit subject
  • used a good, short, descriptive subject
  • explained the problem and solution well in the commit message body
  • wrapped the commit message at 75 characters (line length)

And you get bonus points for adding an extra line right before the Signed-off-by line which explains which other commit was fixed:

Fixes: 2d654c0af194 ("batman-adv: upgrade package to latest release 2012.0.0")

Btw. This repository is not necessarily OpenWrt only. LEDE is also using it (they will also get an own branch for LEDE 17.01 in the next days)

Actions #4

Updated by Sven Eckelmann almost 8 years ago

It looks like you forgot about it. I've now queued up a pull request to fix this problem. https://github.com/openwrt-routing/packages/pull/269

Actions #5

Updated by Sven Eckelmann almost 8 years ago

  • Status changed from In Progress to Resolved

It was merged

Actions #6

Updated by Sven Eckelmann over 7 years ago

  • Status changed from Resolved to Closed
  • Target version set to 2017.0

Closed without a test from reporter because he didn't show any since of life since 25 days.

Actions

Also available in: Atom PDF