Skip to content

Commit

Permalink
C++ style cast required.
Browse files Browse the repository at this point in the history
  • Loading branch information
@dok-net
dok-net committed Nov 14, 2019
1 parent 69d2558 commit a833f27
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions libraries/Ticker/src/Ticker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Ticker
void attach_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg)
{
static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
_attach_ms(milliseconds, true, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
_attach_ms(milliseconds, true, reinterpret_cast<callback_with_arg_t>(callback), reinterpret_cast<void*>(arg));
}

void once(float seconds, callback_function_t callback);
Expand All @@ -73,7 +73,7 @@ class Ticker
void once_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg)
{
static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
_attach_ms(milliseconds, false, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
_attach_ms(milliseconds, false, reinterpret_cast<callback_with_arg_t>(callback), reinterpret_cast<void*>(arg));
}

void detach();
Expand Down

3 comments on commit a833f27

@bkrajendra
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how to make changes in calling code for this change.
its breaking TickerScheduler plugin which uses this library.
Can you please explain how to call this _attach_ms method from usage point of view.?

currently its used as :

template<typename TArg> void attach_ms(uint32_t milliseconds, void(*callback)(TArg), TArg arg)
	{
		this->period = milliseconds;
		this->callback = callback;
		this->callback_argument = arg;
		this->is_attached = true;
	}

Thanks in advanced.

@dok-net
Copy link
Contributor Author

@dok-net dok-net commented on a833f27 Jan 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error: reinterpret_cast from type 'volatile bool*' to type 'void*' casts away qualifiers
I'd say casting away volatile is disregarded at the offender's peril. On the other hand, I've had too many arguments over what people misunderstand or do not misunderstand volatile does to go down this path again.
The change in ESP8266 shows you (or the lib's author) that there is an issue, they should fix that, please. C++ deprecated c-style casts for a reason, a long time ago.
That said, remove volatile from TickerScheduler, also the spurious yield()calls, and test your sketch. I expect no difference but better latency.

@bkrajendra
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dok-net thank you for feedback.
I did as you said.
I removed volatile from all flag instances in code.
and compiled without any issue. more ever its working fine also.
I don't have much knowledge of the internal code flow of TickerScheduler library, hence dont wanted to touch yield() calls
I guess I will leave it upto author of library (if he is active.)

Thanks again for your help.

Please sign in to comment.