Page 1 of 2
chTimeIsInRangeX always false? Topic is solved
Posted: Fri Nov 22, 2019 2:08 pm
by wurstnase
Hi,
I've some issues with the following code:
Code: Select all
/**
* @brief Checks if the specified time is within the specified time range.
* @note When start==end then the function returns always true because the
* whole time range is specified.
*
* @param[in] time the time to be verified
* @param[in] start the start of the time window (inclusive)
* @param[in] end the end of the time window (non inclusive)
* @retval true current time within the specified time window.
* @retval false current time not within the specified time window.
*
* @xclass
*/
static inline bool chTimeIsInRangeX(systime_t time,
systime_t start,
systime_t end) {
return (bool)((systime_t)((systime_t)time - (systime_t)start) <
(systime_t)((systime_t)end - (systime_t)start));
}
When end==start this returns always false, but in the comment it say always true.
When end==start this is always 0. time-start is always > 0, because it's unsigned. In that case 1 < 0 is always false?
In my current specific case:
Code: Select all
chTimeIsInRangeX(9682, 9659, 9659)
Re: chTimeIsInRangeX always false?
Posted: Fri Nov 22, 2019 2:17 pm
by Giovanni
Hi,
Moving in bug reports. I think the documentation is wrong in this case. This code changed a while ago but the documentation note persisted.
Making it as documented would probably require one extra check, it is better to keep this code efficient.
Giovanni
Re: chTimeIsInRangeX always false?
Posted: Fri Nov 22, 2019 7:23 pm
by wurstnase
If I'm correct I could now use in the fallback i2c driver?
Code: Select all
i2cp->start = osalOsGetSystemTimeX();
i2cp->end = i2cp->start + timeout;
/* or i2cp->end = osalTimeAddX(i2cp->start, timeout); */
instead of this "wrong" code?
Code: Select all
i2cp->start = osalOsGetSystemTimeX();
i2cp->end = i2cp->start;
if (timeout != TIME_INFINITE) {
i2cp->end += timeout;
}
Re: chTimeIsInRangeX always false?
Posted: Fri Nov 22, 2019 7:29 pm
by Giovanni
I need to review it, for sure it is not very correct to sum systime_t with sysinterval_t directly.
Giovanni
Re: chTimeIsInRangeX always false?
Posted: Fri Nov 22, 2019 8:03 pm
by wurstnase
Please check also my current patch. (Still testing)
Code: Select all
diff --git a/os/hal/lib/fallback/I2C/hal_i2c_lld.c b/os/hal/lib/fallback/I2C/hal_i2c_lld.c
index 1482d52f5..209506365 100644
--- a/os/hal/lib/fallback/I2C/hal_i2c_lld.c
+++ b/os/hal/lib/fallback/I2C/hal_i2c_lld.c
@@ -94,7 +94,7 @@ static inline msg_t i2c_check_arbitration(I2CDriver *i2cp) {
static inline msg_t i2c_check_timeout(I2CDriver *i2cp) {
- if (!osalOsIsTimeWithinX(osalOsGetSystemTimeX(), i2cp->start, i2cp->end)) {
+ if (!osalTimeIsInRangeX(osalOsGetSystemTimeX(), i2cp->start, i2cp->end)) {
i2c_write_stop(i2cp);
return MSG_TIMEOUT;
}
@@ -105,7 +105,7 @@ static inline msg_t i2c_check_timeout(I2CDriver *i2cp) {
static msg_t i2c_wait_clock(I2CDriver *i2cp) {
while (palReadLine(i2cp->config->scl) == PAL_LOW) {
- if (!osalOsIsTimeWithinX(osalOsGetSystemTimeX(), i2cp->start, i2cp->end)) {
+ if (!osalTimeIsInRangeX(osalOsGetSystemTimeX(), i2cp->start, i2cp->end)) {
return MSG_TIMEOUT;
}
i2c_delay(i2cp);
@@ -347,14 +347,11 @@ void i2c_lld_stop(I2CDriver *i2cp) {
*/
msg_t i2c_lld_master_receive_timeout(I2CDriver *i2cp, i2caddr_t addr,
uint8_t *rxbuf, size_t rxbytes,
- systime_t timeout) {
+ sysinterval_t timeout) {
/* Setting timeout fields.*/
i2cp->start = osalOsGetSystemTimeX();
- i2cp->end = i2cp->start;
- if (timeout != TIME_INFINITE) {
- i2cp->end += timeout;
- }
+ i2cp->end = osalTimeAddX(i2cp->start, timeout);
CHECK_ERROR(i2c_write_start(i2cp));
@@ -399,14 +396,11 @@ msg_t i2c_lld_master_receive_timeout(I2CDriver *i2cp, i2caddr_t addr,
msg_t i2c_lld_master_transmit_timeout(I2CDriver *i2cp, i2caddr_t addr,
const uint8_t *txbuf, size_t txbytes,
uint8_t *rxbuf, size_t rxbytes,
- systime_t timeout) {
+ sysinterval_t timeout) {
/* Setting timeout fields.*/
i2cp->start = osalOsGetSystemTimeX();
- i2cp->end = i2cp->start;
- if (timeout != TIME_INFINITE) {
- i2cp->end += timeout;
- }
+ i2cp->end = osalTimeAddX(i2cp->start, timeout);
/* send start condition */
CHECK_ERROR(i2c_write_start(i2cp));
diff --git a/os/hal/lib/fallback/I2C/hal_i2c_lld.h b/os/hal/lib/fallback/I2C/hal_i2c_lld.h
index 21a7faabf..04614ece7 100644
--- a/os/hal/lib/fallback/I2C/hal_i2c_lld.h
+++ b/os/hal/lib/fallback/I2C/hal_i2c_lld.h
@@ -129,7 +129,7 @@ typedef struct {
/**
* @brief Delay of an half bit time in system ticks.
*/
- systime_t ticks;
+ sysinterval_t ticks;
#else
/**
* @brief Pointer to an externally defined delay function.
@@ -217,10 +217,10 @@ extern "C" {
msg_t i2c_lld_master_transmit_timeout(I2CDriver *i2cp, i2caddr_t addr,
const uint8_t *txbuf, size_t txbytes,
uint8_t *rxbuf, size_t rxbytes,
- systime_t timeout);
+ sysinterval_t timeout);
msg_t i2c_lld_master_receive_timeout(I2CDriver *i2cp, i2caddr_t addr,
uint8_t *rxbuf, size_t rxbytes,
- systime_t timeout);
+ sysinterval_t timeout);
#ifdef __cplusplus
}
#endif
The default delay needs some rework, because it will trigger now a SV#4. The thdSleep is in a locked section.
Re: chTimeIsInRangeX always false?
Posted: Fri Nov 22, 2019 8:10 pm
by Giovanni
Thanks, I must have missed the fallback driver after changing the time API.
Giovanni
Re: chTimeIsInRangeX always false?
Posted: Sun Nov 24, 2019 5:08 pm
by Giovanni
Fixed as bugs #1060 and bug #1061 (trunk only right now).
The problem was present in both RT and NIL, also the OS-less OSAL contained a similar problem. Added specific tests to both test suites.
Do you mind giving a try to the modified I2C driver?
Giovanni
Re: chTimeIsInRangeX always false?
Posted: Mon Nov 25, 2019 10:03 am
by wurstnase
Hi Giovanni,
the driver is working now. Only the default delay is still running into a SV#4.
The i2cMasterTransmit call an osalSysLock.
Then it will start the transmit with i2c_lld_master_transmit_timeout.
E.g. in the i2c_write_start you will have:
Code: Select all
palClearLine(i2cp->config->sda);
i2c_delay(i2cp);
palClearLine(i2cp->config->scl);
where the default i2c_delay is:
Code: Select all
static inline void i2c_delay(I2CDriver *i2cp) {
#if SW_I2C_USE_OSAL_DELAY || defined(__DOXYGEN__)
osalThreadSleep(i2cp->config->ticks);
#else
i2cp->config->delay();
#endif
}
Modifying the code to:
Code: Select all
static inline void i2c_delay(I2CDriver *i2cp) {
#if SW_I2C_USE_OSAL_DELAY || defined(__DOXYGEN__)
osalSysUnlock();
osalThreadSleep(i2cp->config->ticks);
osalSysLock();
#else
i2cp->config->delay();
#endif
}
Could fix it.
Re: chTimeIsInRangeX always false?
Posted: Thu Dec 05, 2019 3:45 pm
by Giovanni
Hi,
I applied a slightly different fix but your solution was correct.
Giovanni
Re: chTimeIsInRangeX always false?
Posted: Thu Dec 05, 2019 6:41 pm
by alex31
Giovanni wrote:Fixed as bugs #1060 and bug #1061 (trunk only right now).
The problem was present in both RT and NIL, also the OS-less OSAL contained a similar problem. Added specific tests to both test suites.
Do you mind giving a try to the modified I2C driver?
Giovanni
Hello,
Theses fixes have the side effect to make chThdSleepUntilWindowed(prev, next) to stuck if prev == next.
Alexandre