Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for PIC32 return types #230

Closed
wants to merge 1 commit into from

Conversation

majenkotech
Copy link

@majenkotech majenkotech commented Aug 8, 2016

This simply abstracts const __FlashStringHelper or const char into a macro CCRET (Const Char RETurn) chosen by the existence of the macro __PIC32__ so the library works properly with the chipKIT system.

@Rotzbua
Copy link
Collaborator

Rotzbua commented Aug 25, 2016

Hi, thanks. But wouldn't it be easier to just use

#ifdef __PIC32__
#define __FlashStringHelper char
#endif

?

@Rotzbua Rotzbua added enhancement 🚀 a wish or proposal for future features need_feedback ↩️ awaiting feedback from people labels Aug 25, 2016
@majenkotech
Copy link
Author

I tried various combinations of that with another library and it completely barfed with overloads where you have a FlashString and a char variant of the same function. In this one specific situation where it's only returning a value it may work better.

@Rotzbua
Copy link
Collaborator

Rotzbua commented Aug 31, 2016

As I mentioned in #216 (comment) . I would like too keep this library as simple as possible. So board/platform specific changes should be done by the Arduino IDE plugin.

Would it be ok, if I mention your compatibility patch for the chipKIT in the readme? @majenkotech

@Rotzbua Rotzbua added the invalid 🚫 not relevant for the library label Aug 31, 2016
@majenkotech
Copy link
Author

Looking at the chipKIT implementation of __FlashStringHelper I am wondering if we actually have it right. We have const char *, which breaks things. Changing it to just plain char fixed your library but breaks overloading in others. One in particular has:

typedef const __FlashStringHelper *FlashString;

And then later has:

void dbg(FlashString str);
void dbg(const char *str);

And they of course crash with each other.

I am thinking switching to char for __FlashStringHelper is probably the best way to go.

@majenkotech
Copy link
Author

I have been having a play with reinterpret_cast and such, and I think I have a more elegant solution. You can close this PR now I guess, it's not going to be needed any more.

@Rotzbua Rotzbua removed enhancement 🚀 a wish or proposal for future features need_feedback ↩️ awaiting feedback from people labels Oct 14, 2016
@spudspud
Copy link

Where do i get codes for mfrc-522

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid 🚫 not relevant for the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants