orgua / OneWireHub

OneWire slave device emulator
GNU General Public License v3.0
343 stars 86 forks source link

implement Resume command 0xA5 #16

Closed vytautassurvila closed 7 years ago

vytautassurvila commented 7 years ago

This PR implements Resume command. Linux 1wire drivers uses it quite often: https://github.com/torvalds/linux/blob/master/drivers/w1/w1_io.c#L441

orgua commented 7 years ago

it is that easy? i would have thought that there are more internal states to handle. looks fine to me

vytautassurvila commented 7 years ago

If I understood documentation correctly there are no interal states to handle, just use previously selected slave. The only place that worries me is https://github.com/orgua/OneWireHub/blob/master/src/OneWireHub.cpp#L468. According to the specs ReadRom should not select slave to be used in ResumeCommand. If I'm correct slave_count only contains count of current OneWireHub slaves but not of whole bus (in case there are serveral OneWireHubs on same line). But probably its bus master responsibility not to issue this command if it has more than one slave on the bus

orgua commented 7 years ago

yeah, i see you know your stuff :-) i was worried there too. and i think we should change it to:

` if (slave_selected == nullptr && slave_count == 1) { slave_selected = slave_list[getIndexOfNextSensorInList()]; } if (slave_selected != nullptr) { slave_selected->sendID(this); };

`

vytautassurvila commented 7 years ago

Well, I was thinking the safest way would be not to assign anything to slave_selected in ReadRom and SkipRom commands. I would probably go for separate variable. On the other hand I'm not sure if we should bother about these two commands using slave_selected and I would guess its bus master's responsibility not to issue these commands if it has more than one slave on the bus (and following commands could mess up ResumeCommand's behavior only if there is more than one slave on the bus).