Skip to contentSkip to conversation

kaffeine 0.8.8 USALS/GotoX problem

PATCH In review Hans Werner 2 participants 2 messages Jul 13, 2009 – Jul 14, 2009 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kaffeine 0.8.8 USALS/GotoX problem

There is a bug in DvbStream::gotoX in Kaffeine 0.8.8, which is fixed by the attached patch.

Look for example at what happens when azimuth = 15.951. The motor should be driven to 16.0 degrees, but when 15.951 is rounded up with
if (USALS>0.5) ++rd;
the carries into the upper nibble of CMD2 and the lower nibble of CMD1 are not handled properly. The result is CMD1 represents 0 instead of 16 degrees and CMD2 is undefined, being set by the non-existent 11th element of DecimalLookup.

The patch fixes it, and I also took the opportunity to fix the strange division by while loops.

Regards,
Hans

Signature · 2 lines
GRATIS für alle GMX-Mitglieder: Die maxdome Movie-FLAT!
Jetzt freischalten unter http://portal.gmx.net/de/go/maxdome01
dvbstream.cpp renamed from dvbstream.cpp.orig +11 -15
@@ -758,30 +758,26 @@
 
 void DvbStream::gotoX( double azimuth )
 {
-	double USALS=0.0;
+	int USALS;
 	int CMD1=0x00, CMD2=0x00;
 	int DecimalLookup[10] = { 0x00, 0x02, 0x03, 0x05, 0x06, 0x08, 0x0A, 0x0B, 0x0D, 0x0E };
 
+	//high nibble of CMD1
 	if ( azimuth>0.0 )
 		CMD1 = 0xE0;    // East
 	else
 		CMD1 = 0xD0;      // West
 
-	USALS = fabs( azimuth );
+	USALS = ( fabs(azimuth)*10 + 0.5 );  // NB tenths of a degree
 
-	while (USALS > 16) {
-		CMD1++;
-		USALS-= 16;
-	}
-	while (USALS >= 1.0) {
-		CMD2+=0x10;
-		USALS--;
-	}
-	USALS*= 10.0;
-	int rd = (int)USALS;
-	USALS-= rd;
-	if ( USALS>0.5 )
-		++rd;
+	//low nibble of CMD1 : 16 degree steps
+	CMD1+=USALS/160;
+
+	//high nibble of CMD2 : 1 degree steps
+	CMD2=((USALS%160)/10)*0x10;
+
+	//low nibble of CMD2: 0.1 degree steps, coded as in DecimalLookup
+	int rd  = USALS%10;
 	CMD2+= DecimalLookup[rd];
 
 	rotorCommand( 12, CMD1, CMD2 );
#1 lore raw

Hello Hans,

I looked into the patch - just to be informed what's going on and got confused by such code.
I reimplemented it to test it against side effects and still do not understand why it is written so complicated. Sure, I don't know the environment of the code, but does it matter in this case ?

If you like try these few lines (the last 3 lines) as a replacement:

	unsigned long USALS;
	int CMD1;
	int CMD2;

if ( azimuth>0.0 )

		CMD1 = 0xE0;    // East
 	else
		CMD1 = 0xD0;    // West

/*-----------------------------------------------*/

	USALS = ( fabs( azimuth ) * 16.0 + 0.5 );
	CMD1 |= ( USALS & 0xf00 ) >> 8;
	CMD2  = ( USALS & 0x0ff );

/*-----------------------------------------------*/

Regards
/Jesko

-----Ursprüngliche Nachricht-----
Von: [email protected] [mailto:[email protected]] Im Auftrag von Hans Werner
Gesendet: Dienstag, 14. Juli 2009 01:21
An: [email protected]
Betreff: [BUG][PATCH] kaffeine 0.8.8 USALS/GotoX problem

There is a bug in DvbStream::gotoX in Kaffeine 0.8.8, which is fixed by the attached patch.

Look for example at what happens when azimuth = 15.951. The motor should be driven to 16.0 degrees, but when 15.951 is rounded up with if (USALS>0.5) ++rd; the carries into the upper nibble of CMD2 and the lower nibble of CMD1 are not handled properly. The result is CMD1 represents 0 instead of 16 degrees and CMD2 is undefined, being set by the non-existent 11th element of DecimalLookup.

The patch fixes it, and I also took the opportunity to fix the strange division by while loops.

Regards,
Hans

--
GRATIS für alle GMX-Mitglieder: Die maxdome Movie-FLAT! Jetzt freischalten unter http://portal.gmx.net/de/go/maxdome01