Re: [PATCH] Fix access point rate for trunk and stable

[Helmut Schaa <hschaa suse de>]

Hi Dan,

attached is an adopted version of my patch using a uint32 for the rate 
property. Please have a look at my inline comments.

Am Montag, 8. Oktober 2007 15:58:20 schrieb Dan Williams:
> Yeah, I think I'd prefer that.  If we ever get a rate out of
> wpa_supplicant that is < 0, NM should just clamp to 0.  I think this
> would only happen in error conditions, and with the D-Bus interface
> errors are reported through exceptions anyway.
>
> So making it UINT32 would be great.
>
> As for the divisor, the reason wext and wpa_supplicant (which just
> passes back what wext tells it) use large numbers is because Jean was
> trying to write wext for a wide range of hardware, which is fine.  But
> it turns out that it's only used for 802.11 hardware really, which means
> a certain number of rates starting at 1Mb/s.  There does need to be at
> least .1Mb/s resolution.  So I propose the following:
>
> 1) Make it a uint32
> 2) clamp the int to (0, INT_MAX) and cast to uint32 (ie, leave the value
> as-is with out dividing by 1000000)

The division by 1000000 is only used in nm-tool to convert the rate to Mb/s.

> There certainly are bitrates of 5.5 Mb/s for example, that we need to
> make available, and the way it was done before with an integer and
> dividing by 1000000 meant that could not be done.

Thanks,
Helmut

--- src/NetworkManagerAP.c
+++ src/NetworkManagerAP.c
@@ -40,7 +40,7 @@
 	int				mode;		/* from IW_MODE_* in wireless.h */
 	gint8			strength;
 	double			freq;
-	guint16			rate;
+	guint32			rate;
 	guint32			capabilities;
 
 	/* Non-scanned attributes */
@@ -365,14 +365,14 @@
  * Get/set functions for rate
  *
  */
-guint16 nm_ap_get_rate (const NMAccessPoint *ap)
+guint32 nm_ap_get_rate (const NMAccessPoint *ap)
 {
 	g_return_val_if_fail (ap != NULL, 0);
 
 	return (ap->rate);
 }
 
-void nm_ap_set_rate (NMAccessPoint *ap, guint16 rate)
+void nm_ap_set_rate (NMAccessPoint *ap, guint32 rate)
 {
 	g_return_if_fail (ap != NULL);
 
--- src/NetworkManagerAP.h
+++ src/NetworkManagerAP.h
@@ -66,8 +66,8 @@
 double			nm_ap_get_freq			(const NMAccessPoint *ap);
 void				nm_ap_set_freq			(NMAccessPoint *ap, double freq);
 
-guint16			nm_ap_get_rate			(const NMAccessPoint *ap);
-void				nm_ap_set_rate			(NMAccessPoint *ap, guint16 rate);
+guint32			nm_ap_get_rate			(const NMAccessPoint *ap);
+void				nm_ap_set_rate			(NMAccessPoint *ap, guint32 rate);
 
 gboolean			nm_ap_get_invalid		(const NMAccessPoint *ap);
 void				nm_ap_set_invalid		(NMAccessPoint *ap, gboolean invalid);
--- test/nm-tool.c
+++ test/nm-tool.c
@@ -187,7 +187,7 @@
 		temp = g_strdup_printf ("%s Mode, Freq %.3f MHz, Rate %d Mb/s, Strength %d%%%s%s",
 						    (mode == IW_MODE_INFRA) ? "Infrastructure" : "Ad-Hoc", 
 						    flt_freq,
-						    rate / 1024,
+						    rate / 1000000,
 						    strength,
 						    (enc_string && strlen (enc_string->str)) ? enc_string->str : "",
 						    !broadcast ? ", Hidden" : "");

Index: test/nm-tool.c
===================================================================
--- test/nm-tool.c	(Revision 3020)
+++ test/nm-tool.c	(Arbeitskopie)
@@ -125,7 +125,7 @@
 							(nm_access_point_get_mode (ap) == IW_MODE_INFRA) ? "Infra" : "Ad-Hoc",
 							nm_access_point_get_hw_address (ap),
 							nm_access_point_get_frequency (ap),
-							nm_access_point_get_rate (ap) / 1024,
+							nm_access_point_get_rate (ap) / 1000000,
 							nm_access_point_get_strength (ap));
 
 	if (   !(flags & NM_802_11_AP_FLAGS_PRIVACY)
Index: src/NetworkManagerAP.c
===================================================================
--- src/NetworkManagerAP.c	(Revision 3020)
+++ src/NetworkManagerAP.c	(Arbeitskopie)
@@ -43,7 +43,7 @@
 	int				mode;		/* from IW_MODE_* in wireless.h */
 	gint8			strength;
 	guint32			freq;		/* Frequency in GHz * 1000; ie 2.412 == 2412 */
-	guint16			rate;
+	guint32			rate;
 
 	guint32			flags;		/* General flags */
 	guint32			wpa_flags;	/* WPA-related flags */
@@ -965,9 +965,9 @@
  * Get/set functions for rate
  *
  */
-guint16 nm_ap_get_rate (NMAccessPoint *ap)
+guint32 nm_ap_get_rate (NMAccessPoint *ap)
 {
-	guint16 rate;
+	guint32 rate;
 
 	g_return_val_if_fail (NM_IS_AP (ap), 0);
 
@@ -977,7 +977,7 @@
 }
 
 void
-nm_ap_set_rate (NMAccessPoint *ap, guint16 rate)
+nm_ap_set_rate (NMAccessPoint *ap, guint32 rate)
 {
 	NMAccessPointPrivate *priv;
 
Index: src/NetworkManagerAP.h
===================================================================
--- src/NetworkManagerAP.h	(Revision 3020)
+++ src/NetworkManagerAP.h	(Arbeitskopie)
@@ -93,8 +93,8 @@
 guint32			nm_ap_get_freq			(NMAccessPoint *ap);
 void				nm_ap_set_freq			(NMAccessPoint *ap, guint32 freq);
 
-guint16			nm_ap_get_rate			(NMAccessPoint *ap);
-void				nm_ap_set_rate			(NMAccessPoint *ap, guint16 rate);
+guint32			nm_ap_get_rate			(NMAccessPoint *ap);
+void				nm_ap_set_rate			(NMAccessPoint *ap, guint32 rate);
 
 gboolean			nm_ap_get_invalid		(const NMAccessPoint *ap);
 void				nm_ap_set_invalid		(NMAccessPoint *ap, gboolean invalid);
Index: libnm-glib/nm-access-point.c
===================================================================
--- libnm-glib/nm-access-point.c	(Revision 3020)
+++ libnm-glib/nm-access-point.c	(Arbeitskopie)
@@ -534,7 +534,7 @@
 		 g_param_spec_uint (NM_ACCESS_POINT_RATE,
 						"Rate",
 						"Rate",
-						0, G_MAXUINT16, 0,
+						0, G_MAXUINT32, 0,
 						G_PARAM_READWRITE));
 
 	g_object_class_install_property