Refactor pc_tranc.c
- split sync and async code in
pc_tranc.cinto different functions. -
pc__trans_resp,pc__trans_sentandpc__trans_fire_eventwill not queue items, thependingarg has removed. - do state change in
pc_trans_fire_event, that is when event occurs, the state is changed not when the event is dispatched. ( this shuld fixes #60 ) - using a simple state machine for state translate code in
pc_trans_fire_event.
TODO: remove extra indent for code marked with // follow code has extra indent for better git diff in pull request. after the pr is merged.
@cynron Any feedbacks?
sorry, I am too busy these days, I will review this heavy PR carefully later.
@xpol
1. split sync and async code in pc_tranc.c into different functions.
2. pc__trans_resp, pc__trans_sent and pc__trans_fire_event will not queue items, the pending arg has removed.
I think this change is not neccessary as the orignal works well, but even so, I think it should be in a seperated commit
3. do state change in pc_trans_fire_event, that is when event occurs, the state is changed not when the event is dispatched. ( this shuld fixes #60 )
awesome, as we had not used poll mode heavily, the poll mode is not well-tested in our use case. it is a bug, good catch
4. using a simple state machine for state translate code in pc_trans_fire_event.
this looks good, but I think the impl can be better:
- macro
RETURN_IFusing VAR_ARGS which may not work on some old compilers ? - more comment on the state trans table ?
-
state_trans_table_entrymay be a better name thanstate_tans_s? - represent
state_trans_s.allowdin bits not an array? I thinkshortis long enough - some syntax error in error message ?
Thank you!
represent state_trans_s.allowd in bits not an array? I think short is long enough
I think its just use some bytes, but provide more readability and simple code.
I think this change is not neccessary as the orignal works well, but even so, I think it should be in a seperated commit
IMHO, works well is not sufficient reason for doing sync and async in one single function. And I agree, that it should go to a seprated PR ( #66 ).
#66 is merged, awesome