[updating subject for my own sanity]

On 02/08/2013 08:55 PM, Scott Talbert wrote:
> Yep, disconnect between brain and keyboard.
> 
>> +   // Skip 1st two bytes, read up to packet length.
>> +   for (int i=2; i<len+2; i++) {
>>
>> Do you mean len-2?
> 
> No, the first two bytes are not included in "len" so we read a full "len" 
> of bytes from 2 -> len+2.  Comment added to clarify.

Ah, right.

> 
>> +int CRemoteMH::SetTime(const TRemoteInfo &ri, const THarmonyTime &ht,
>> + lc_callback cb, void *cb_arg, uint32_t cb_stage)
>> +{
>> + return 0;
>> +}
>>
>> Why isn't this LC_ERROR_UNSUPP?
> 
> I think it was because SetTime() gets called by some of the higher level 
> operations (update config, firwmare) and if it returns non-success, it 
> causes the higher level operations to report failure.  I'll add a comment.

Out of curiosity is this a "we don't know how to do it" or a "you don't set
the time" on these guys?

>> +     /* 2nd parameter is the number of packets remaining,
>> +        plus one */
>> +     if (pkts_to_send < 50)
>> +       ack_rsp[7] = pkts_to_send + 1;
>>
>> And if it's more than 50? You don't fill it in at all?
> 
> No, it's initialized to 0x33 before the loop:
>       uint8_t ack_rsp[MH_MAX_PACKET_SIZE] =
>               { 0xFF, 0x03, 0x00, 0x02, 0x01, 0x05, 0x01, 0x33 };

Or rather, if it's > 50, we just leave it at 50?

> 
>> + const uint8_t finish_msg[MH_MAX_PACKET_SIZE] = {
>>
>> Jesus, these guys aren't playing around...
> 
> What do you mean?

I don't remember what I was commenting on. I think it's because we send an
end_msg, but then a finish_msg... it seemed verbose.

> 
>> + debug("msg_5");
>> + print_packet(rsp);
>> +
>> + cb(LC_CB_STAGE_FINALIZE_UPDATE, cb_count++, 3, 4,
>> +   LC_CB_COUNTER_TYPE_STEPS, arg);
>> +
>> + /* write msg 7 */
>>
>> Huh? what happened to 6?
> 
> In this particular case, "msg 7" refers to the message having 0x07 as the 
> second byte.  Unfortunately I was not very creative in naming messages.
> 
> Updated patch attached to bug report.

Man... I feel like we should be able to abstract all of the IR stuff out...
all the copy-pasta makes me sad.

+ // This section lifted from LearnIR() in remote.cpp
+ // Only difference is that MH IR sequence doesn't start at zero.

Can we can just make LearnIRInnerLoop(int seq) and then make both LearnIR
functions call that one with the right argument?

Then minor style thing... I've tried to move all of the assignments to be 'foo
= bar' instead of 'foo=bar'. A grep shows I have more to do, but can you
update your for loops? Stuff like:

  for (int i=2; i<len+2; i++) {

should be:

  for (int i = 2; i < len + 2; i++) {

-- 
Phil Dibowitz                             p...@ipom.com
Open Source software and tech docs        Insanity Palace of Metallica
http://www.phildev.net/                   http://www.ipom.com/

"Be who you are and say what you feel, because those who mind don't matter
 and those who matter don't mind."
 - Dr. Seuss


Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Own the Future-Intel(R) Level Up Game Demo Contest 2013
Rise to greatness in Intel's independent game demo contest. Compete 
for recognition, cash, and the chance to get your game on Steam. 
$5K grand prize plus 10 genre and skill prizes. Submit your demo 
by 6/6/13. http://altfarm.mediaplex.com/ad/ck/12124-176961-30367-2
_______________________________________________
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel

Reply via email to